Responsive/adaptive website code

I have all these functions that work together to create functionality on a page. Is the structure of these functions OK? Can I do anything to speed this up or make my code better? I'm not exactly sure what namespaces are, but might they help me here?

This code is for a responsive/adaptive calendar website. On the homepage, there is a calendar with events listed by day. These functions filter the events using the ISOTOPE plugin, open and close dropdowns, etc.

If anyone is really interested. I can send you a link + login to the PW protected site.

```
var $eventwrap = $j('.tw-lists'),
$daywrap = $j('.tw-day'),
$dayfilter = $j('#tw-filter-days li a');
$daywraphide = $j('tw-day.hide'),
$catwrap = $j('.tw-list-filter'),
$viewctrls = $j('.tw-view a'),
$selectdays = $j('.select-days option'),
$selectbarrio = $j('.select-barrio option'),
$selectcats = $j('.select-cats option');

filters = {};

var opday = [],
opcategory = [],
opbarrio = [];

function optionArray(select,options) {
$j(select).find('option').each(function() {
options.push($j(this).attr('data-filter'));
});

return options;
}

// CHECK IF A GIVEN DAY HAS EVENTS
function filterToggle(element,x,y) {
element.each(function(){
var $me = $j(this),
isli = $me.is('li');

if(isli) {
var myvalue = $me.find('a').attr('data-filter');
} else {
var myparent = $me.parent().attr('data-filter-group'),
myvalue = $me.attr('data-filter');
}

if(!x) {x = ''}
if(!y) {y = ''}

var eventcount = $j('.tw-list'+ myvalue + x + y).length;

if(eventcount == 0) {
if(isli) {
$me.addClass('empty tdn');
} else {
$me.attr('disabled','disabled');
}
} else {
if(isli) {
$me.removeClass('empty tdn');
} else {

Here are the first steps I would take (not necessarily in order):

-
Surround everything with an anonymous self calling function

(function () {
    ...
}());

This both helps ensure you don't accidentally add/overwrite global variables and aids in the future minification of your script.

-
Define all variables at the start of their respective functions

if(isli) {
    var myvalue = $me.find('a').attr('data-filter');
} else {
    var myparent = $me.parent().attr('data-filter-group'),
        myvalue = $me.attr('data-filter');
}

This kind of code is error-prone and makes future refactorings harder (and you will change it later).

-
Explicitly export any globals you wish to define at the very end of your script

$.extend(window, {
    'scrollableSetup': scrollableSetup,
    'filterClick': filterClick,
    'switchViews': switchViews,
    'mobileFocus': mobileFocus,
    'filterDrop': filterDrop,
    'openDropDowns': openDropDowns
});

This also helps the minification process (internally the function names can be munged). As a general rule it is better to be explicit where you can be. The quotes are important to certain minifiers.

-
Explicitly declare/pass in any global objects to the anonymous function

(function (window, document, $, ...) {
    ...
}(window, document, jQuery, ...));

Again this helps minification (now globals can be munged). Also, still better to be explicit.

-
Run JSLint on your code

http://www.jslint.com/

The only errors I tolerate in JSLint are:

• messy whitespace (sometimes)

• ++ usage (only in counter part of for loops)

• "'variable' was used before it was defined." (only in the parameter list to the anonymous method)

-
Remove unused variables

$select.focus(function(){
    var $me = $j(this),
        previousval = $me.value,
        $previous = $me.find(':selected');
}).change(function(){

Unused variables are dead weight. Every line of code you don't need to write is a line of code that the person looking at this 6 months from now doesn't need to look at and figure out what you were thinking. Unused variables come in several forms:

• variables in the beginning of your functions (see step 2) that don't get used within the function

• variables that get exported to an outer scope but do not serve a purpose

• functions that do not get used

• event handlers that don't do anything

-
Code that does the same thing should look the same

JQuery has a half dozen ways to bind events (.bind, .on, .click, .live, .delegate, .one). The only one you need is .on. Replace:

.bind('click', function () {

with

.on('click', function () {

and so on.

Similarly within a method, avoid switching between if statements and switch statements and back:

if (x === y) {
    return 'x';
}
switch (x) {
case z:
    ...
}
if (x ...

This is better written with pure if statements.

Continuing this thought process, don't switch between method short circuiting and "The One Return" (with one exception: arg checks):

if (x) { return a; }
if (y) { return b; }
if (z) { r = ... }
if (!r && t) { r = ... }
...
return r || ...;
}

In general, logic that changes in style is much harder to read than logic that remains stylistically constant.

-
Pull constants out

If a variable doesn't change within a function or between runs of a function, then it is not a variable with respect to that function. Thus it doesn't belong there (except at the root level).

-
Use the right method for the job

Instead of                                   Use 
.attr('data-x')                              .data('x')
if (x) { $a.show() } else { $a.hide() }      $a.toggle(x) 
x ? $a.addClass('a') : $a.removeClass('a')   $a.toggleClass('a', x)

StackExchange Code Review Q#11233, answer score: 24