CODEWITHSUNDEEP
javascript
Kyle Hotchkiss
Kyle Hotchkiss

Reputation: 11112

Is this piece of Javascript inefficient?

I was just wondering whether this snippet of Javascript is going to slow down my site:

$(function(){
 var realLink = location.href;
 $( "#nav a" ).each(
  function( intIndex ){
   String.prototype.startsWith = function(str){
    return (this.indexOf(str) === 0);
   }
   var pageLink = $(this).attr("href");

   if ( realLink.startsWith(pageLink) )
    $(this).parent().addClass("active");   
  }
 );
});

It only loops about 5-7 times, and I don't have very much Javascript looping experience.

Upvotes: 0

Views: 208

Answers (5)

Dan Beam
Dan Beam

Reputation: 3927

if you use the ^= operator in Sizzle (well, jQuery in this case) it means "starts with this value" basically. try this:

$(function(){
   $( '#nav a:not( [href^="' + startsWith + '"] )' ).addClass( 'active' );
});

and change startsWith to a variable of your choice.

Upvotes: 0

Tobias Cohen
Tobias Cohen

Reputation: 20000

Try using Firebug's Profiling Feature to measure the performance of your script.

Upvotes: 1

kangax
kangax

Reputation: 39168

There's nothing inherently wrong with this snippet, except that you're constantly creating and assigning a function to String.prototype.startsWith in a loop. That, of course, is an unnecessary work, and should at least be:

$(function(){
 var realLink = location.href;
 String.prototype.startsWith = function(str){
   return (this.indexOf(str) === 0);
 };
 $( "#nav a" ).each(
  function( intIndex ){
   var pageLink = $(this).attr("href");
   if ( realLink.startsWith(pageLink) )
    $(this).parent().addClass("active");   
  }
 );
});

I also don't see a need for intIndex argument there. It's not used anywhere in a function.

Upvotes: 5

Justin Niessner
Justin Niessner

Reputation: 245429

You could make that code perform a little better by changing it to:

String.prototype.startsWith = function(str){
    return(this.indexOf(str) == 0);
}

$(function(){
    var realLink = location.href;

    $('#nav a').each(function(intIndex){
        var pageLink = $(this).attr("href");
        if(realLink.startsWith(pageLink))
            $(this).parent().addClass("active");
        });
});

It just breaks out the definition of startsWith() so you don't define it for each iteration of the loop.

Upvotes: 2

Igor Zevaka
Igor Zevaka

Reputation: 76510

Should be fine. If the #nav element doesn't contain thousands of links you should be OK.

Install Firebug for Firefox and you can do a profile run. It will tell you how much time it spends in each function. My guess it will be in milliseconds for this case.

Upvotes: 0

Related Questions