views:

130

answers:

1

Perhaps I am doing this wrong and suggestions on how to improve my code are appreciated. My situation is this: I have a toolbar with different elements that are populated by a callback. I do not want to use the show() or hide() commands, I prefer to use detach, but I think there should be a nice way to deal with it. Here's my code:

entryView = function _entryView() {

    var menuButton = $('<div/>').addClass('menuButton');        

    toolBar();

    $.getJSON('ajax', function(o) {
        var $enum2 = ss.IEnumerator.getEnumerator(dto.TransmittalDates);
        while ($enum2.moveNext()) {
            var dateTime = $enum2.get_current();
            $('.menu').append($('<div/>').addClass('menuitem').text(dateTime.toString()));
        }
    });
}
    toolBar = function _toolBar() {
    var flyoutMenu = $('<div/>').addClass('menu');

    $('.menuButton').click(function(o) {
          $('.menubutton').append(flyoutMenu);
    });

I did a quick cut and paste and renamed the variables to make them make sense. As you can see on the entry I build the toolbar and the very last thing I do is the ajax call. The menu, however, is not created until the "click" event, so appending is not possible.

I realize that having global variables is bad, so I'm trying to avoid that, but I think the best situation would have the ajax call populate a Menu variable and when the DOM is created, to pull from that same Menu item. How do I pull this off? Is there a better way to do it?

Edit: Fubbed a bit on the toolbar function, I think I have it should be correct now.

A: 

I'm confused by some parts of your code:

  • What's the entryView function and when is it called?
  • Why does the toolBar function exist as opposed to being inline? From where else is it called?
  • Why are you creating functions like that? Creating a variable without var is bad practice, always makes global variables, and will be forbidden in ES5 strict mode. You should create functions like this:

    var someFunction = function(arg1, arg2) { … };
    

    or like this:

    function someFunction(arg1, arg2) { … }
    
  • Why are you giving each function a second name (e.g. _toolBar)? The "private" name will only be in scope inside the function.

The menu doesn't have to be in global scope, just in a scope that's common to both functions.

I would refactor it like this (knowing very little about the design of your application), inlining toolBar:

function entryView() {
    var menuButton = $('<div/>' {'class': 'menuButton'}), menu = $('<div/>', {'class': 'menu'});

    $.getJSON('ajax', function(o) {
        var $enum2 = ss.IEnumerator.getEnumerator(dto.TransmittalDates);
        while ($enum2.moveNext()) {
            var dateTime = $enum2.get_current();
            $('.menu').append($('<div/>' {'class': 'menuItem'}).text(dateTime.toString()));
        }
    });

    menuButton.click(function(o) {
          menuButton.append(menu);
    });
}
Sidnicious
also, won't this `$('.menu').append(...` not actually act on the `div` with the class `menu` that he created and is storing in the `var menu` ? it should be `$('.menu', menu).append(...` ...I think (again, having hardly the faintest idea what the intentions are here).
samandmoore