Using a variable for a jQuery object rather than rebuilding it for different function calls is a performance enhancement. There were many cases where when given an already constructed jQuery object the results of get(0) on that object was stored and then a new jQuery object created from that variable. The other place was where $(this) was being used multiple times in a single function.

This patch means that the overhead of building the jQuery object is only needed once.

Comments

james.elliott’s picture

Assigned: Unassigned » james.elliott
Status: Active » Needs review
quicksketch’s picture

Status: Needs review » Needs work

This seems mostly okay with me. I tend to use the raw DOM objects when possible because it makes for easier comparisons and it is sometimes more efficient when pulling out raw attribute (i.e. element.id or element.className), but for the items you've selected it may make sense. However before making this change I'd like to be consistent about our variable names. It's a common jQuery convention to name jQuery object variables with a $ prefix, so instead of element, if it's a jQuery object it should be $element. Then you know the difference between variables that are jQuery objects and those that are not.

james.elliott’s picture

The $ prefix is a convention that some adopt. I personally don't use it because I like to keep a clear separation when I'm working with variables in JS or PHP, lest I fall into the habit of not using var for JS variable declarations. However, I don't feel strongly about it either way and am happy to use the $ prefix.

quicksketch’s picture

Status: Needs work » Fixed
StatusFileSize
new8.76 KB

This patch had some pretty major issues (lots of undefined variables, using jQuery methods on non-jQuery objects). Rerolled with consistent $-prefixing of jQuery objects and corrected all JS errors. Same patch applied to both branches.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.