Improving JavaScript - using objects vs. just functions

Question

I'm no stranger to JS, but I think I still have a lot to learn. I originally wrote this code to manipulate a "help" bar, which would be positioned as a thin ribbon on the side of the browser window, which when hovering over or clicking would open up and display it's contents. Nothing major, but an okay example to use here.

$(document).ready(function () {
var open = false;
var peek = false;
var helpBar = $(".help-bar");
var helpIcon = $(".help-icon");
var help = helpBar.add(helpIcon); // Put both together to do the same things

helpBar.click(function () {
    toggleHelp();
});

helpBar.hover(function () {

    togglePeek();
});

function toggleHelp() {
    if (open) {
        changeRightValue(help, "0", "slow");
        toggleArrow("Open");
        open = false;
    }
    else {
        changeRightValue(help, "280", "fast");
        toggleArrow("Close");
        open = true;
    }
}

function togglePeek() {
    if (open == false) {
        help.stop();
        if (peek) {
            changeRightValue(help, "0", "fast");
            peek = false;
        }
        else {
            changeRightValue(help, "5", "fast");
            peek = true;
        }
    }
}

function changeRightValue(element, amount, speed) {
    element.animate({ right: amount }, speed);
}

function toggleArrow(direction) {
    helpIcon.css("background", "url('/images/buttons/zapp/arrow" + direction + ".png') no-repeat 0px 50%");
}
});

It works fine, but then I wondered whether grouping related items into objects might help a little:

    $(document).ready(function(){
    var open = false;
    var peek = false;

    var helpBar = $(".help-bar");
    var helpIcon = $(".help-icon");

    var helpSection = helpBar.add(helpIcon); // Put both together to do the same things

    var help = {};
    var utils = {};

    help.toggle = function(){
        if (open) {
            utils.changeRightValue(helpSection, "0", "slow");
            utils.toggleArrow("Open");
            open = false;
        }
        else {
            utils.changeRightValue(helpSection, "280", "fast");
            utils.toggleArrow("Close");
            open = true;
        }
    };

    help.peek = function(){
        if (open == false) {
            helpSection.stop();
            if (peek) {
                utils.changeRightValue(helpSection, "0", "fast");
                peek = false;
            }
            else {
                utils.changeRightValue(helpSection, "5", "fast");
                peek = true;
            }
        }
    };

    utils.changeRightValue = function(element, amount, speed){
        element.animate({right:amount}, speed);
    };

    utils.toggleArrow = function(direction){
        helpIcon.css("background", "url('/images/buttons/zapp/arrow" + direction + ".png') no-repeat 0px 50%");
    };

    // jQuery DOM
    helpBar.click(function () {
        help.toggle();
    };

    helpBar.hover(function () {

        help.peek();
    };
});

Is this that much of an improvement over the first? Is there another way that I should be writing this kind of code?

Answer 1

If this is already your entire code, then you won't benefit much from namespacing your functions. Namespacing is basically assigning stuff to an object property. Accessing object properties is a minor overhead, but an overhead nevertheless.

Also, it doesn't look like anything else is accessing the code made for click and hover, so I'd rather place them directly into the handler instead.

You should be fine with simple functions in this case.

I'd rather concentrate more on reducing redundant code. Here's what I have done:

//$(document).ready(fn) can be shortened to $(fn)
$(function () {

  //declare variables in a function block up top
  //this is how the compiler sees them, so to avoid unexpected behavior
  //just follow suit
  var open = false
    , peek = false
    , helpBar = $(".help-bar")
    , helpIcon = $(".help-icon")
    , help = helpBar.add(helpIcon)
    ;

  //as explained, namespacing isn't an advantage at this level of code
  //and so, I have removed the functions and placed the code into the handlers

  //you can modify "this" in a function by using .call() or .apply()
  //in this case, element argument is removed and will be passed as "this"
  //this method will be explained later
  function changeRightValue(amount, speed) {
    this.animate({
      right: amount
    }, speed);
  }

  function toggleArrow(direction) {
    helpIcon.css("background", "url('/images/buttons/zapp/arrow" + direction + ".png') no-repeat 0px 50%");
  }

  //here we have our merged functions

  helpBar.on('click',function () {

    //if blocks that call the same functions but differ in parameters are messy
    //what I did here is made ternary ifs to pick arguments, not pick invocations
    //the calls will be made later
    var changeRightParams = open ? [0, 'slow'] : [280, 'fast']
      , direction = open ? 'Open' : 'Close'
      //this is a very short way to toggle booleans in the same variable
      , open = !open
      ;

    //here we call the functions with the "picked" arguments

    //here we use .apply to set the invoked function's this as help
    //and the arguments as the picked arguments
    //it's the same as calling "help.changeRightValue(amount,speed)
    changeRightValue.apply(help, changeRightParams);
    toggleArrow(direction);

  }).on('hover',function () {

    //like above, declare variables up top to avoid unexpected behavior
    var changeRightParams;

    //here we use the "early return" pattern, to avoid nesting ifs
    //since nothing executes when open is true, we return early
    //we saved 1 indent level in the code
    if (open) return;

    //code after the early return assumes "open" is false
    //it would be similar to if(!open){/*stuff here*/}

    help.stop();

    //same as above, we pick arguments, toggle the boolean, call with the selected
    changeRightParams = peek ? [0, 'fast'] : [5, 'slow'];
    peek = !peek
    changeRightValue.apply(help, changeRightParams);

  });
});

Answer 2

A couple of pointers not in any particular order:

If you use a single main object with methods it'll be easier to follow along later and add functionality etc. A pattern you can use is the observer pattern for setting up and triggering your events. Also if you use a single main object you can refer to the methods with this.someMethod() or to config options like this.someOption.

Another thing is when you initialize your variables, you don't have to set them to false. If you do var yolo; it will be set to a falsy value so all you have to do later is set to true, or to test for if false use !yolo. On the other hand if you want to test for true just test for (yolo) like you have been doing. That way you can set any value to yolo and the test will be ok.

Answer 3

As long as help.toggle and help.peek does not use this, you can simplify callback:

// jQuery DOM
helpBar.click( help.toggle );

helpBar.hover( help.peek );

And you can also chain it together

// jQuery DOM
helpBar.click( help.toggle )    
       .hover( help.peek );