The Elements of JavaScript Style (Part Two)

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 ? '#fff' : '#989898';
    }
    p = d.getElementById('e' + (em - 1));
    if (p) {
        p.style.backgroundColor = z ? '#fff' : '#989898';
    }
}

---

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.

原文The Elements of JavaScript Style