The Elements of JavaScript Style

Part Two: Idioms

Douglas Crockford
The Department of Style
2005-09-21

There are idioms that we can use to make our intentions clearer and more concise.

Consider this function:

function gw(f) {
    if (d.w.sv.checked == true) {
        zv = 'on';
    } else {
        zv = 'off';
    }
    procframe.location.replace("http://b.www.yahoo.com/module/wtr_tr.php?p=" + 
        escape(f.p.value) + "&sv=" + zv);
    return false;
}

The == operator should not be used for comparing values with true because it does type coercion. If our intent is to determine if d.w.sv.checked is the boolean value true, then we must use the === operator. If we only care that a value is truthy (and not falsy) then it is better to not use an equality operator at all.

For example, because of type coercion., 1 == true is true, but 1 === true is false. The == operator can hide type errors.


Watch out for type coercion when using ==.


The if statement is being used to select one of two values. This is what the ?: ternary operator is for.

zv = d.w.sv.checked ? 'on' : 'off';

Use the ?: operator to select one of two values.


The variable zv is not declared as a var or parameter of this function, so it is an implicit global variable. If there is another function on this page that uses a similarly named global variable, then a failure could result. Bugs like this can be very difficult to find but are very easily avoided. In this case, we can either declare that zv as a var, or we can notice that it is used only once and get rid of it entirely.

function gw(f) {
    procframe.location.replace("http://b.www.yahoo.com/module/wtr_tr.php?p=" + 
        escape(f.p.value) + "&sv=" + (d.w.sv.checked ? 'on' : 'off'));
    return false;
}

Never use implicit global variables.


We would normally be suspicious of functions that return a constant, but this is something that is sometimes required in a browser environment.

Next we see a case where the ?: operator is used improperly. It is being used to select between two assignments.

function u(o, z) {
    var em = o.id.substring(1);
    var p = d.getElementById('e' + em);
    if (p) {
        (z == 0) ? p.style.backgroundColor = '#fff' : 
                   p.style.backgroundColor = '#989898';
    }
    p = d.getElementById('e' + (em - 1));
    if (p) {
        (z == 0) ? p.style.backgroundColor = '#fff' : 
                   p.style.backgroundColor = '#989898';
    }
}

The test of z is ambiguous. Do we select color #fff if z is exactly 0, or if z is falsy? As stated it appears to indicate the former, but it actually means the latter. Fortunately in this case, we probably intend the latter, so it is not technically an error (this time). But it is bad stylistically.

We can replace the ?: with an if, but it happens that the assignments all use the same lvalue, so this time we can make the correction without using an if.

function u(o, z) {
    var em = o.id.substring(1),
        p = d.getElementById('e' + em);
    if (p) {
p.style.backgroundColor = z ? '#989898' : '#fff';
}
p = d.getElementById('e' + (em - 1));
if (p) {
p.style.backgroundColor = z ? '#989898' : '#fff';
} }

Do not use the ?: operator to select one of two actions.


Event handling suffers from browser dependencies. Ideally, application programs should be insulated from browser deficiencies by common libraries. When such libraries are not available, functions like this happen:

function md(e) {
    (window.event) ? ev = window.event : ev = e;
    (ev.target) ? sr = ev.target : sr = ev.srcElement;
    if (ev && sr && sr.id == "fp" || sr.id == "sb") st = 1;
    if (sr.className.indexOf("pllist") < 0 && sr.className != "more" && 
            sr.className != "plinkc" && sr.tagName != "scrollbar " && 
            _toClose && _toCloseNorgie) {
        d.getElementById(_toClose).innerHTML = "";
        _toClose = "";
        _toCloseNorgie.parentNode.className = '';
        _toCloseNorgie = '';
    }
}

Some browsers pass an event object to event handlers as a parameter. Microsoft chose instead to put the event object in a global event variable. In JavaScript, global variables are members of the global object. In browsers, the global object always contains a window member whose value is the global object. Accessing global variables through window is a way of avoiding undefined variable errors when testing for the existence of a variable. However, it should never be necessary to make such a test.

Instead of first determining if this is a Microsoft event, we can instead ask if it is the other kind.

ev = e || event;

We used the || (default) operator. If e is truthy, we will use its value, but if e is falsy then we will use event.

In the next statement, we can again use the || operator to determine sr, the source element or target.

We should make ev and sr vars to avoid global conflict.

function md(e) {
    var ev = e || event,
        sr = ev.target || ev.srcElement;
    if (sr && (sr.id == 'fp' || sr.id == 'sb')) {
        st = 1;
    }
    if (sr.className.indexOf('pllist') < 0 && sr.className != 'more' && 
            sr.className != 'plinkc' && sr.tagName != 'scrollbar ' && 
            _toClose && _toCloseNorgie) {
        d.getElementById(_toClose).innerHTML = '';
        _toClose = '';
        _toCloseNorgie.parentNode.className = '';
        _toCloseNorgie = '';
    }
}

Use the || operator to specify a default value.


Next we find another event handler. As you would expect, it repeats some of the same stylelessness as the previous one.

function kd(e) {
    (window.event) ? ev = window.event : ev = e;
    (ev.target) ? el = ev.target : el = ev.srcElement;
    if (ev && el) {
        code = ev.keyCode; 
        id = el.id;
    } else {
        return;
    }
    ctn = lt.id.substring(1);
    if (code == 13) {
        return;
    } else if ((code == 191 || code == 222) && id != 'fp') {
        _ffs = 1;
        gk = 0;
    } else if ((code < 31 || code > 41) && 
            (code < 16 || code > 18) && code != 9 && code != 8) {
        gk = 1;
    } else {
        gk = 0;
    }
    if (!_ffs && (id == 'fp' || id == 'st')) {
        if (code == 9) {
            if (box.value == '' || (box.value != '' && (at == 1 || ev.shiftKey))) {
                mt(ctn);
            } else if (id == 'st' && box.value != '' && at == 0) {
                at = 1;
                mt(ctn);
            }
        } else if (id == 'fp' && gk == 0 && 
                 (box.value == '' && st == 0) && !ev.shiftKey && !ev.ctrlKey && !ev.altKey) {
            d.getElementById('mk').focus();
            d.getElementById('mk').blur();
        } else if (gk == 1) {
            at = 0;
        }
    } else if ((id == 'mk2' && box.value != '' && ev.shiftKey && code == 9) || 
            (id == 'm6' && !ev.shiftKey && code == 9)){
        d.getElementById('mk').focus();
    } else if (!_ffs && gk == 1 && el.type != 'text' && !ev.ctrlKey && !ev.altKey){
        box.value = '';
        box.focus();
    }
}
function mt(ctn) {
    if ((ev && !ev.ctrlKey && !ev.altKey) || !ev) {
        if (ev.shiftKey){
            nextTab = parseInt(ctn) - 1;
        } else {
            nextTab = parseInt(ctn) + 1;
        }
        if (nextTab == 0) {
            d.getElementById('mk').focus();
        } else if (nextTab < 8) {
            t(d.getElementById('v' + nextTab));
        } else {
            return;
        }
    }
}

What is interesting is that it has a companion function, mt, which is only called from kd. mt is passed one parameter (ctn), but most of the communication between kd and mt is through global variables.


Global variables are evil.


We could eliminate the use of global variables by increasing the number of parameters sent to mt. But instead, we will make mt an inner function of kd. As an inner function, mt would have access to all of kd's vars.

function kd(e) {
    var ev = e || event,
        el = ev.target || ev.srcElement,
        cnt,
        code = ev.keyCode,
        gk,
        id = el.id,
        ctn = lt.id.substring(1);
    
    function mt() {
        var nextTab;
        if (!ev.ctrlKey && !ev.altKey) {
            nextTab = parseInt(ctn) + ev.shiftKey ? -1 : 1;
            if (!nextTab) {
                d.getElementById('mk').focus();
            } else if (nextTab < 8) {
                t(d.getElementById('v' + nextTab));
            }
        }
    }
    
    if (code == 13) {
        return;
    } else if ((code == 191 || code == 222) && id != 'fp') {
        _ffs = 1;
        gk = 0;
    } else if ((code < 31 || code > 41) && 
            (code < 16 || code > 18) && code != 9 && code != 8) {
        gk = 1;
    } else {
        gk = 0;
    }
    if (!_ffs && (id == 'fp' || id == 'st')) {
        if (code == 9) {
            if (box.value == '' || 
                    (box.value != '' && (at == 1 || ev.shiftKey))) {
                mt();
            } else if (id == 'st' && box.value != '' && at == 0) {
                at = 1;
                mt();
            }
        } else if (id == 'fp' && gk == 0 && (box.value == '' && st == 0) && 
                !ev.shiftKey && !ev.ctrlKey && !ev.altKey) {
            d.getElementById('mk').focus();
            d.getElementById('mk').blur();
        } else if (gk == 1) {
            at = 0;
        }
    } else if ((id == 'mk2' && box.value != '' && ev.shiftKey && code == 9) || 
            (id == 'm6' && !ev.shiftKey && code == 9)){
        d.getElementById('mk').focus();
    } else if (!_ffs && gk == 1 && el.type != 'text' && !ev.ctrlKey && 
            !ev.altKey) {
        box.value = '';
        box.focus();
    }
}

Function mt is called from two places in kd. By making it an inner function, we were able to significantly reduce the number of global variables that kd uses, which reduces its likelihood of interfering with other components. kd is still a mess, but it is now a slightly less disorderly mess.


Use inner functions to avoid global variables.