Running with Code Like with scissors, only more dangerous

6Jan/120

Defining Variables in JavaScript

I’ve lately been reviewing different patterns and practices recently, and after reading his article about the Revealing Prototype Pattern, I wanted to take some time to analyze Dan Wahlin’s approach to defining variables. It’s hard to believe I found some common ground with Douglas Crockford, but as they say about broken clocks…. [Addendum: apparently, since JSlint says to 'combine with previous var statement,' I don't agree with Crockford.] Anyway, to begin, this post is inspired by Dan Wahlin’s presentation of the Revealing Prototype Pattern; I noticed what I thought was a curious way for him to define his private variables, and looking back through his blog posts he discussed it in the original blog post of the series, Techniques, Strategies, and Patterns for Structuring JavaScript Code. For the most part, I like what Dan has to say, but I’m going to have to disagree when it comes to defining variables.

The Proposition

As Dan points out, this is the standard way of defining variables in JavaScript:

var eqCtl;
var currNumberCtl;
var operator;
var operatorSet = false;
var equalsPressed = false;
var lastNumber = null;

He advocates trading that for this:

var eqCtl,
    currNumberCtl,
    operator,
    operatorSet = false,
    equalsPressed = false,
    lastNumber = null;

It saves on 20 keystrokes, and he claims improved readability. Now, I disagree with Crockford’s argument that, because JavaScript hoists variables to the top of the function, that you should always declare the variable there. I believe that, whenever possible, you should try to maximize locality of a variable. This is a principle discussed in Steve McConnell’s Code Complete; the reasoning behind maximization of locality is that the human brain can only comprehend so much at once. (This is, of course, another argument in favor of many, simple, and small subroutines). By delaying the declaration of a variable until it needs to be used, we are able to better-comprehend the meaning of the variable and how its use affects and relates to the rest of the program. As such, I believe that one of the premises for moving these declarations into a combined var statement – specifically, to reflect the hoisting – is a poor rationale.

Let’s carry on.

Similarities to Other Elements

In The Prototype Pattern, Dan demonstrates the use of a JavaScript object literal in assignment to the Calculator prototype, so that any object created using the Calculator constructor would inherit all of those properties:

Calculator.prototype = {
    add: function (x, y) {
        return x + y;
    },
    subtract: function (x, y) {
        return x - y;
    },
    multiply: function (x, y) {
        return x * y;
    },
    // ...
};

The important thing to note here is that we are simply defining an object literal; we are not writing procedural code, and that comma is not an operator! It is an important part of the JavaScript grammar, to be sure, but the comma here does not have the same semantic meaning as the comma we saw before. This subtle difference may lead to coding errors, in which someone who uses comma syntax with both will mistakenly believe they are declaring an object literal and use colons to separate the identifier from the value; or that they are declaring variables and use assignment syntax to separate the property from its value.

Comma Operator Considered Harmful

It surprises me to learn that JSlint advocates combining var declarations. Crockford’s The Elements of JavaScript Style, Part 1 indicates that he isn’t terribly fond of it either:

The comma operator was borrowed, like much of JavaScript’s syntax, from C. The comma operator takes two values and returns the second one. Its presence in the language definition tends to mask certain coding errors, so compilers tend to be blind to some mistakes. It is best to avoid the comma operator, and use the semicolon statement separator instead.

Whichever way Crockford prefers it, I think what we need to remember is just because you CAN do something does not mean you SHOULD.

Let’s consider Dan’s full body of JavaScript from the Revealing Prototype Pattern. I’m going to shrink it a little bit, to emphasize the changes I’ll make; and I’m removing any of his comments.

var Calculator = function (cn, eq) {
    this.currNumberCtl = cn;
    this.eqCtl = eq;
};

Calculator.prototype = function () {
    var operator = null,
        operatorSet = false,
        equalsPressed = false,
        lastNumber = null,
        add = function (x, y) { return x + y; },
        subtract = function (x, y) { return x - y; },
        multiply = function (x, y) { return x * y; },
        // I'm going to do something evil here.
        divide = function (x, y) {
            if (y == 0) {
                alert("Can't divide by 0");
            }
            return x / y;
        },
        setVal = function (val, thisObj) { thisObj.currNumberCtl.innerHTML = val; },
        setEquation = function (val, thisObj) { thisObj.eqCtl.innerHTML = val; },
        clearNumbers = function () {
            lastNumber = null;
            equalsPressed = operatorSet = false;
            setVal('0',this);
            setEquation('',this);
        },
        setOperator = function (newOperator) {
            if (newOperator == '=') {
                equalsPressed = true;
                calculate(this);
                setEquation('',this);
                return;
            }
            if (!equalsPressed) calculate(this);
            equalsPressed = false;
            operator = newOperator;
            operatorSet = true;
            lastNumber = parseFloat(this.currNumberCtl.innerHTML);
            var eqText = (this.eqCtl.innerHTML == '') ?
                lastNumber + ' ' + operator + ' ' :
                this.eqCtl.innerHTML + ' ' + operator + ' ';
            setEquation(eqText,this);
        },
        numberClick = function (e) {
            var button = (e.target) ? e.target : e.srcElement;
            if (operatorSet == true || 
                this.currNumberCtl.innerHTML == '0') {
                setVal('', this);
                operatorSet = false;
            }
            setVal(this.currNumberCtl.innerHTML + button.innerHTML, this);
            setEquation(this.eqCtl.innerHTML + button.innerHTML, this);
        },
        calculate = function (thisObj) {
            if (!operator || lastNumber == null) return;
            var displayedNumber = parseFloat(thisObj.currNumberCtl.innerHTML),
                newVal = 0;
            switch (operator) {
                case '+':
                    newVal = add(lastNumber, displayedNumber);
                    break;
                case '-':
                    newVal = subtract(lastNumber, displayedNumber);
                    break;
                case '*':
                    newVal = multiply(lastNumber, displayedNumber);
                    break;
                case '/':
                    newVal = divide(lastNumber, displayedNumber);
                    break;
            }
            setVal(newVal, thisObj);
            lastNumber = newVal;
        };
    return {
        numberClick: numberClick,
        setOperator: setOperator,
        clearNumbers: clearNumbers
    };
} ();

Note my comment: “I’m going to do something evil here.” Here goes:
console.log('Hello, world')

Do you see what happened here? Let me put it in context.

        subtract = function (x, y) { return x - y; },
        multiply = function (x, y) { return x * y; },
        console.log('Hello, world')
        divide = function (x, y) {
            if (y == 0) {
                alert("Can't divide by 0");
            }
            return x / y;
        },

JavaScript semicolon insertion blew away the var when I inserted any valid statement or expression. In fact, I could have simply put 'Hello, world!' or 5 there, on its own line, and because the next line is a valid statement that stands on its own, JavaScript semicolon insertion blew away the var. As such, divide, setVal, setEquation, clearNumbers, setOperator, numberClick, and calculate were all just elevated to global scope, possibly blowing away existing variables and leaking a whole bunch of information with them. This could happen in any instance in which someone mistakenly types a semicolon (let’s be honest – JavaScript is a semicolon-terminated language; it will happen somewhat frequently), or if they forget to put a comma at the end of a line.

As such, joining variable declarations together by using the comma operator is inherently an unsafe operation. You might think of it as a run-on sentence; it’s not a good thing to do in English, so why would it be good to do in JavaScript or any other programming language?

And if that’s not reason enough, here’s another: declaring a variable is a statement. You are stating to the compiler, “I am declaring this variable to be a variable.” Use the var statement to make a statement, and use the comma operator to indicate that there are operations. (Specifically, the one and only place I can think of in which a comma operator would be appropriate is if you need a single for-loop with multiple iterator variables, e.g., for (var i = 0, j = myArray.length - 1; i = 0; i++, j--). Of course, I don’t want to say you should never use it, or else I’d be like Crockford with his dogmatic “I have never seen a piece of code that was not improved by refactoring it to remove the continue statement,” which is patently silly.

But, beware the comma. He is correct in that it is easy to mark programming errors with commas. If you’re going to declare a variable, do everyone a favor and declare it, using var, make it feel special by giving it its own line and declaration and semicolon. It will help in maintenance down the line.

Comments (0) Trackbacks (1)

Leave a comment

ERROR: si-captcha.php plugin says GD image support not detected in PHP!

Contact your web host and ask them why GD image support is not enabled for PHP.

ERROR: si-captcha.php plugin says imagepng function not detected in PHP!

Contact your web host and ask them why imagepng function is not enabled for PHP.