WebDevStudios / CollabPress

74 stars 35 forks source link

Update frontend.js #66

Closed mikeburns closed 11 years ago

mikeburns commented 11 years ago

On pages where there is no #datepicker element, it throws a JS error. I'm not deep enough into the project in order to find a better solution. I'm thinking there should be a LOT more code for frontend.js - and should also take into account internationalisation (custom date formats)

ericandrewlewis commented 11 years ago

Hi Mike, thanks for the bug report and pull request.

Instead of a try...catch clause, a better plan would be not to fire the JS if the element isn't on the page.

mikeburns commented 11 years ago

sounds good. I tend to be super stingy when it comes to memory allocation. I've worked on several rather large projects in the past where the many thousands of JavaScript modules slowed down the browser considerably (mainly IE ;) and since then I've really looked into making JS code as memory-friendly as possible. For instance:

var x=$('#element');
x.doAction();
x.doAction1();
x.doAction2();
x.doAction3();

is much more efficient than

$('#element').doAction();
$('#element').doAction1();
$('#element').doAction2();
$('#element').doAction3();

because we don't have to search the DOM each time, and only allocate one variable. Therfore:

if($('#element').length > 0) $('#element').doAction();

searches the DOM twice, whereas:

try { $('#element').doAction();} catch(e) {}

only does it once. Another alternative is:

var x=$('#element');
if(x) x.doAction();

but we then have an extra cycle or two for the branch. I've taken to using try...catch in similar situations where we had like 3,000 DOM objects that could or could not exist, and had to iterate over them in loops, handling each case separately. FF and IE had some troubles, as they reached their memory limits and didn't do garbage collection properly, so I only defined the variables once and simply changed them out for each loop, or just put a try...catch around especially computationally-intensive areas, so that I could save like 20 computations per loop because the error was caught on the first line and then it simply did the next iteration. 20 x 3,000 = 60,000 executed lines, probably each with a few computational steps.

Sorry, just wanted to share some "wisdom" I've gathered over the years. I love encountering such "border" cases. Through these I think we really see where performance and good coding style can play huge roles.

Peace

ericandrewlewis commented 11 years ago

Hi Mike,

definitely appreciate your thoughts on JS/jQuery efficiency. In 591ba792816e4c9c2fe14a44cc7f60f55266dff9, I've actually removed frontend.js as it was some cruft left over from version 1.2- that we actually aren't using, and sadly was enqueued improperly on every page load.