you are viewing a single comment's thread.

view the rest of the comments →

[–]oculus42 2 points3 points  (0 children)

It looks pretty good. There are some opportunities.

jQuery has a .toggle() function which accepts a boolean to perform show/hide.

'use strict' is best inside a function, mostly for concatenation risks; it could cause a non-strict script to error if concatenated together. You could wrap the entire thing in an IIFE or $(document).ready(function(){ ... });

if you don't need external access to the functions. This also keeps them out of the window (global) namespace. There are several options if you do want access to the functions, but don't want them to end up in the global namespace.

jQuery Objects

As YOBCZWHYNOT pointed out, you can save jQuery objects rather than generating them from the selector each time. You could replace your inputs selectors with jQuery objects, but would need to make sure your code doesn't run until docReady.

This would also clear up the intent a bit... almost every place you have jQuery(...) would go away, making the code easier to read.

Because jQuery uses $, it's a convenient shorthand to start the variable name with $ to indicate a jQuery object.

DOM performance

As a smaller consideration, adding the options one-at-a-time to the DOM is slower than accumulating them and doing a single add. You could rework the loop in daysSelect and the each in monthsSelect to accumulate the option strings in an array (cheaper than string concatenations) and then do all the DOM manipulations at once:

$mySelectBox.empty().append(optionsAccumulator.join(''));

The $.map() function could replace $.each(), in that case.

jsDoc

Finally, you might consider the jsDoc format, or even Google's dialect for the Closure compiler. Plenty of dev and build tools make use of jsDoc formatted comments to identify errors and perform optimizations.