JavaScript Concern from a recent review

Myk Melez myk at mozilla.org
Mon Apr 9 15:05:06 UTC 2007


Gervase Markham wrote:
> I don't believe that omitting braces adds to readability. The most it 
> does, given Bugzilla's current coding style, is save a line.
In this case, omitting braces actually saves three lines, and it would 
be even clearer if the comment commented the entire block to which it 
applies.

The original:

for (var i = 0; i < document.forms.length; i++) {
    for (var j = 0; j < document.forms[i].elements.length; j++) {
    /* MS decided to add fieldsets to the elements array; and
     * Mozilla decided to copy this brokenness. Grr.
     */
        if (document.forms[i].elements[j].tagName != 'FIELDSET') {
            document.forms[i].elements[j].onmouseover = showHelp;
        }
    }
}


My nit:

for (var i = 0; i < document.forms.length; i++)
    for (var j = 0; j < document.forms[i].elements.length; j++)
        /* MS decided to add fieldsets to the elements array; and
         * Mozilla decided to copy this brokenness. Grr.
         */
        if (document.forms[i].elements[j].tagName != 'FIELDSET')
            document.forms[i].elements[j].onmouseover = showHelp;


What I consider optimal:

// MS decided to add fieldsets to the elements array; 
// and Mozilla decided to copy this brokenness. Grr.
for (var i = 0; i < document.forms.length; i++)
    for (var j = 0; j < document.forms[i].elements.length; j++)
        if (document.forms[i].elements[j].tagName != 'FIELDSET')
            document.forms[i].elements[j].onmouseover = showHelp;


> And I don't think that advantage outweighs the potential mistakes that 
> can be made in maintenance, and the mental switch required to notice 
> the different syntax in this special case.
It's not a particularly special case, as one-line blocks occur 
frequently in code, and the "mental-switch" required is IMHO less 
significant than that required to grok a condition/block combination on 
a single line.

-myk




More information about the developers mailing list