Brian Coolidge
Brian Coolidge

Reputation: 4649

Jquery Best Practice - Putting DOM Element in Variable regardless of how many times it was used

While browsing for some best practices on Jquery, I stumbled into this one.

Reference: http://gregfranko.com/jquery-best-practices/#/13

// Stores the live DOM element inside of a variable
  var elem = $("#elem");

  // Set's an element's title attribute using it's current text
  elem.attr("title", elem.text());

  // Set's an element's text color to red
  elem.css("color", "red");

  // Makes the element fade out
  elem.fadeOut();

It says it's the best to CACHE YOUR SELECTORS IN VARIABLES

Then while working, I thought to myself, do I really have to put #current-contact-info and #list-contact-form on a variable?

Since I'm just calling it once, does it affect the performance?

Option 1:

$("#edit-contact-list").on("click", function() {
    $("#current-contact-info").addClass("hide");
    $("#list-contact-form").removeClass("hide");
});

Or option 2 is the best way regardless of how many times it was called?

Option 2:

$("#edit-contact-list").on("click", function() {

    var current_contact_info = $("#current-contact-info");
    var list_contact_form = $("#list-contact-form");

    current_contact_info.addClass("hide");
    list_contact_form.removeClass("hide");
});

I was kind of insecure on the amount of line of code though.

Upvotes: 0

Views: 52

Answers (2)

Ajinkya
Ajinkya

Reputation: 22720

Always consider simplicity and readability along with performance/best practices. Storing a element in variable is of no use if it will be used only once, also it wont help in making code more simple/readable.
I always prefer

 $("#current-contact-info");.addClass("hide");
 $("#list-contact-form").removeClass("hide");

over

var current_contact_info = $("#current-contact-info");
var list_contact_form = $("#list-contact-form");

current_contact_info.addClass("hide");
list_contact_form.removeClass("hide");

If you are worried about lines of code (it deosnt really matter though), you can achieve it in 1 line. Use toggleClass over addClass/removeClass and use OR operator in selector

 $("#current-contact-info, #list-contact-form").toggleClass("hide");

Upvotes: 2

Christophe
Christophe

Reputation: 28154

"cache" implies the intention to reuse the data later. If you use it only once, this is not needed.

In your case however, it is not clear how many times the click might occur. If more than once, then you could consider caching your elements as follows (outside the click event handler):

var current_contact_info = $("#current-contact-info");
var list_contact_form = $("#list-contact-form");

$("#edit-contact-list").on("click", function() {
  current_contact_info.addClass("hide");
  list_contact_form.removeClass("hide");
});

Upvotes: 2

Related Questions