views:

251

answers:

7

As a rule of thumb, which of these methods of writing cross-browser Javascript functions will perform better?

Method 1

function MyFunction() 
{
    if (document.browserSpecificProperty)
       doSomethingWith(document.browserSpecificProperty);
    else
       doSomethingWith(document.someOtherProperty);
}

Method 2

var MyFunction;
if(document.browserSpecificProperty) {
    MyFunction = function() {
       doSomethingWith(document.browserSpecificProperty);
    };
} else {
    MyFunction = function() {
       doSomethingWith(document.someOtherProperty);
    };
}

Edit: Upvote for all the fine answers so far. I've fixed the function to a more correct syntax.

Couple of points about the answers so far - whilst in the majority of cases it is a fairly pointless performance enhancement, there are a few reasons one might want to still spend some time analyzing the code:

  • Has to run on slow computers, mobile devices, old browsers etc.
  • Curiosity
  • Use the same general principal to performance enhance other scenarios where the evaluation of the IF statement does take some time.
+16  A: 

Unless you're doing this a trillion times, it doesn't matter. Go with the one that is more readable and maintainable to you and/or your organization. The productivity gains you will get from writing clean, simple code matters way more than shaving a tenth of a microsecond off your JS execution time.

You should only even start thinking about what performs better when and only when you've written code and it is unacceptably slow. Then you should start tracking down the bottleneck, which will never be something like this. You will never get a measurable performance gain out of switching from one to the other here.

Rex M
You're the man Rex +1
alex
+1  A: 

Technically, I would say that the second one would perform better, because the if statement is only executed once, rather than every time the function is run.

The difference, however, would be negligible to the point of being meaningless. The performance penalty of a single if statement such as this would be insignificant even compared to the performance penalty of simply calling a function. It would make a smallish difference even if if is called a million times.

The first one is easier to understand, because it doesn't have the awkwardness of defining the same function twice based on a condition, with both versions behaving differently. That seems to be a recipe for confusion later on.

I wouldn't be the first person to say that unless you are really insane about this optimization thing, you'll get more of a win out of code readability.

thomasrutter
Not sure why this is getting so many downvotes - if you think I'm wrong could you please explain why.
thomasrutter
+7  A: 

Unfortunately the code above is not actually cross-browser friendly as it relies on a mozilla quirk not present in other browsers -- namely that function statements are treated as function expressions inside branches. On browsers other that aren't built on mozilla the above code will always use the second function definition. I made a simple testcase to demonstrate this here.

Basically the ECMAScript spec says that function statements are treated similarly to var declarations, eg. they all get hoisted to the top of the current execution scope (eg. the start of a <script> tag, the start of a function, or the start of an eval block).

olliej
+3  A: 

To clarify olliej's answer, your second method is technically a syntax error. You could rewrite it this way:

var MyFunction;
if(document.browserSpecificProperty) {
    MyFunction = function() {
       doSomethingWith(document.browserSpecificProperty);
    };
} else {
    MyFunction = function() {
       doSomethingWith(document.someOtherProperty);
    };
}

Which is at least correct syntax, but note that MyFunction would only be available in the scope in which that occurs. (Omit var MyFunction;, and preferably use window.MyFunction = function() ... for global.)

eyelidlessness
actually it's not a syntax error, and is perfectly valid js, it's just not likely to do what you wanted. This is much like the `var` behaviour, in that people expect js to use syntactic scoping for vars but it actually just scopes to functions.
olliej
From my understanding, it is in fact a syntax error. Perhaps my understanding is wrong. I would have to reread the relevant parts of the spec, and unfortunately I can't right now. I'll remove that portion of my answer.
eyelidlessness
I shall recheck the spec -- i know all JS engines will parse it, so it may have become defacto syntax, even if it doesn't do what's wanted.
olliej
Ah ha, you are right, per spec it would be a parse error, as the FunctionDeclaration production only occurs inside the SourceElement production rather than statement. So I apologise you were right -- it is technically a syntax error.
olliej
Rolled back. I remember reading a critique of Prototype which described it as a syntax error, so I had looked it up in the spec and concluded that they were right (on that point). But my actual memory of the spec had faded. Thanks for checking before I got back to it. :)
eyelidlessness
Does the person who downvoted this answer want to explain why?
eyelidlessness
A: 

I generally prefer the second version, as the condition only has to be evaluated once and not on every call, but there are times when it's not really feasible because it will hamper readability.

Btw, this is a case where you might want to use the ?: operator, e.g (taken from production code):

var addEvent =
    document.addEventListener ? function(type, listener) {
        document.addEventListener(type, listener, false);
    } :
    document.attachEvent ? function(type, listener) {
        document.attachEvent('on' + type, listener);
    } :
    throwError;
Christoph
A: 

For your simplified example I would do what's below assuming that your browser property check only needs to be done once:

var MyFunction = (function() {

      var rightProperty = document.browserSpecificProperty || document.someOtherProperty;

      return function doSomethingWith() {
            // use the rightProperty variable in your function
      }
})();
nickyt
A: 

The performance should be nearly equal!

Thing about using Frameworks like JQuery to get rid of the Browser compability problems!

If performance is your main goal, have a look at SlickSpeed! It is a page which benchmarks different JavaScript frameworks!

Martin K.