Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am using jQuery 1.7.2

I have two ways of writing the same code. One way seems more verbose and the other seems more flexible. Is this the only difference? Is there any performance difference? Is there a better way of writing these?

Code sample #1 uses two named functions so they are reusable. It's a bit more code.

Code sample #2 uses two unnamed functions that work only on the hover method. It's less code but not reusable.

Any thoughts or comments?

//  CODE SAMPLE #1

// IPP FUNCTIONS    
var openIPP = function openIPP() {
    $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
}
var closeIPP = function closeIPP() {
    $IPPNonDefault.stop().animate({"height": "0px"}, UpRate);   
}

// IPP ACTIONS
$IPPWrapper.mouseover(openIPP).mouseout(closeIPP);

//  CODE SAMPLE #2

$IPPWrapper.hover(
    function() {
        $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
    },
    function() {
        $IPPNonDefault.stop().animate({"height": "0px"}, UpRate);
    }
);
share|improve this question
2  
If you have to re-use them, use named functions. If you won't be re-using them, there's no point in using named functions (other than for debugging purposes). – Joseph Silber Sep 5 '12 at 17:54
1  
Where you're creating a function, use either function foo() {} or var foo = function () {};, not var foo = function foo() {};. The last method can have some very odd side-effects in IE<9, and is more prone to typos. – st-boost Sep 5 '12 at 23:57
@st-boost, this is an answer (advice), not a comment. If you copy your comment into an answer box, I will happily vote it up. Thanks! – Evik James Sep 6 '12 at 16:36
@EvikJames thanks, but I think an answer should be a thorough analysis of the problem, and I just wanted to drop a tip. I'm happy with "useful comment", thanks. – st-boost Sep 6 '12 at 19:08
@JosephSilber also, re-using doesn't necessarily mean writing in multiple places. For example, a function should (almost) never be created in a loop. – st-boost Sep 6 '12 at 19:10
show 2 more comments

2 Answers

So this is redundant.

var openIPP = function openIPP() {
    $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
}

Because both of these functions are the same.

var openIPP = function() {
    $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
}
function openIPP() {
    $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
}

This is the easiest for me to read.

$IPPWrapper.hover(
    function() {
        $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
    },
    function() {
        $IPPNonDefault.stop().animate({"height": "0px"}, UpRate);
    }
);

But if you want more information in case a bug is thrown then use named functions like so.

$IPPWrapper.hover(
    function openIPP() {
        $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
    },
    function closeIPP() {
        $IPPNonDefault.stop().animate({"height": "0px"}, UpRate);
    }
);
share|improve this answer

Generally speaking, anonymous (unnamed) functions are completely acceptable within the land of Javascript. They are used frequently and without prejudice, so don't be afraid of them.

That said, I'm not a fan of the second code sample. Without an understanding of the jQuery API, I don't really know what the two functions do to the hover call. Are both called, one after another? Perhaps something else happens? The name hover doesn't really give any clues here.

I think a great option here would be to combine the two samples. Use the anonymous functions of the second with the explicit naming of the first. Furthermore, it should be noted that according to the docs, hover is shorthand for mouseenter and mouseleave, not mousein and mouseout. I'll let you read over the docs and decide which of the two you decide to go with, but ultimately I recommend going with something like this:

$IPPWrapper.mouseenter(function() {
    $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
}).mouseleave(function() {
    $IPPNonDefault.stop().animate({"height": "0px"}, UpRate);
});
share|improve this answer
Danny, the hover method does exactly what the mouseenter and mouseleave methods do. It just makes it simpler. Again, my functions above both do the exact same thing, just through a different means. Your code actually confuses the code a bit. It's actually less flexible and more code. Yikes! – Evik James Sep 5 '12 at 18:44
Perhaps you missed the point of my question, but I was asking if there was any benefit other than reusability. You did, however, address that. I try to make my code reusable so try to use named functions. – Evik James Sep 5 '12 at 18:46

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.