tags:

views:

136

answers:

1

Why is jQuery.lint.js saying that I've used the same selector more than once?

Here's my code:

var seconds = 20;

function updateCountdown() {
    if (seconds === 0) {
        document.forms[0].submit();
    } else {
        $("#countdown").text("Refresh: " + seconds);
        seconds = seconds - 1;
    }
}

jQuery(function($) {
   setInterval("updateCountdown()",1000);
});

It's saying:

Location:

@http://localhost/js/SubmitForm_Countdown.js:7

@http://localhost/js/SubmitForm_Countdown.js:13

Selector: "#countdown"

+5  A: 

I would guess it is referring to $("#countdown").text(...). You're running that same selector once every second.

It would be more efficient to cache it in a variable, and reference it that way.

var seconds = 20;
var $countdown;   // variable to store #countdown. 
                  // Initialized here so it is in scope of updateCountdown()

function updateCountdown() {
    if (seconds === 0) {
        document.forms[0].submit();
    } else {
           // reference the #countdown element stored in the variable
        $countdown.text("Refresh: " + seconds);
        seconds = seconds - 1;
    }
}

jQuery(function($) {
      // Once document is ready, find the #countdown element, and cache it
   $countdown = $('#countdown');
   setInterval("updateCountdown()",1000);
});

Also, it could be argued that it is better/more efficient to pass the named reference of the updateCountdown function to the setInterval() instead of a String.

   setInterval(updateCountdown,1000);

Also, it doesn't appear as though you're clearing your setInterval() once seconds reaches 0. Probably a good idea.

var seconds = 20;
var $countdown;   // variable to store #countdown. 
                  // Initialized here so it is in scope of updateCountdown()

var interval; // store reference to the setInterval()

function updateCountdown() {
    if (seconds === 0) {
        document.forms[0].submit();
           // clear the setInterval
        clearInterval( interval );
    } else {
           // reference the #countdown element stored in the variable
        $countdown.text("Refresh: " + seconds);
        seconds = seconds - 1;
    }
}

jQuery(function($) {
      // Once document is ready, find the #countdown element, and cache it
   $countdown = $('#countdown');
      // retain reference to the setInterval call
   interval = setInterval(updateCountdown,1000);
});
patrick dw
This is good advice, from an efficiency point of view, but I'd be impressed if jQuery.lint is doing this kind of analysis. Not having used jQuery.lint, I'd guess that it's doing static analysis, not runtime analysis, which would be necessary to catch the situation that you describe (Or am I wrong about that). If Phillip reports that the suggestion fixes his problem, then I will consider myself 'impressed'.
belugabob
@belugabob - Here's a simple test (open your console to view). http://jsfiddle.net/DPcjK/ It appears as though that is exactly what jQuery Lint is doing. If you modify the example to cache the selector, the warning disappears.
patrick dw
Caching the selector worked! Thanks Patrick!http://pastebin.com/5YzE81v4
cf_PhillipSenn
Patrick - as promised, I am impressed. I now need to investigate the use of jQuery.lint myself. Thanks for the info.
belugabob