We have been developing some coding style guide lines to produce better code and make our code easier maintainable and portable. Also thinking about code inspection tools and continuous integration techniques which analyze the code will profit from a well defined and verifyable style.
While discussing a rather simple rule I thought this might be interesting to delve into a bit. You will be surprised how much you can discuss about a simple thing such as a for-loop and that is not only true for JavaScript, I am sure.
Well, call me pedantic for even discussing this and seeing this as relevant. But everybody knows that adhering coding styles make programming in a team way more pleasant and it is just way more efficient to just code and not to style! Which includes not having to think about how to style the code, though I realize that a lot of programmers have a problem adapting to coding styles.
We at uxebu basically defined that we are applying the coding guide lines of the project that we possibly contribute stuff back to. This means our JavaScript code applies the dojo coding style guide and our python stuff (we use internally) applies the django coding style guide.
But let’s look at the for-loop now.
The most common usage of a for loop is the following.
1 2 3 4 | for (var i=0; i<nodes.length; i++){ var node = nodes[i]; // whatever } |
Nothing spectacular here. But let’s take it apart a bit. There are two things that are commonly known that can be optimized.
So let’s optimize the things we just found out.
1 2 3 4 5 6 | var node; var len=nodes.length; for (var i=0; i<len; i++){ node = nodes[i]; // whatever } |
Another step on our way was to optimize the variable declarations and reducing their count from three down to one. In some code you can then see the following.
1 2 3 4 5 | var node, i=0, len=nodes.length; for (; i<len; i++){ node = nodes[i]; // whatever } |
Actually we have already skipped a tiny step in between, very often you can see multiple var-statements as opposed to the already “squeezed” one-liner in line 1 above. The way of writing it as seen here, is reasonable if there are a couple other variables to be declared, you just need one var-statement. This will basically reduce the code size, if your minifier doesn’t already do that for you. As stated here it seems to be specified to be irrelevant to performance. So event the argument under 2. against the first version (see above) is not very strong.
Just a small side-note: the one line var-statement evaluates in declaration order, so something like this will work!
1 | var arr = [1,2,3], len = arr.length, x = len-1; |
The “len” and also “x” variables contain the proper content as you are expecting. The comma-separated var-statement evaluates the assignment expression by expression and steps on to the next assignment. Therefore “len” and “x” will have the proper values 3 and 2.
But let’s go back to our for-loop.
If the var-statement is outside the for-loop and especially if other variables (outside of the scope of the for-loop) are declared there the source code get’s blury and harder to read. For example:
1 | var someVar = whatever, anotherVar = [1,4,5], node, i=0, len=nodes.length; |
The first two variable declarations (someVar and anotherVar) have nothing to do with the for-loop context, but the others do, so this blurs the source code and is prone to errors. Also if you separate this source code onto multiple lines like so:
1 2 | var someVar = whatever, anotherVar = [1,4,5], node, i=0, len=nodes.length; |
it is still a possible point of failure. I for example, forgot the comma on line 1 which made this source code
1 2 | var someVar = whatever, anotherVar = [1,4,5] node, i=0, len=nodes.length; |
still work. But and this is a big but, the code get’s a different meaning now! First, line 1 will work, since the semicolon at the end of the line is not mandatory. And second (if not declared inside this scope before) the variables on line 2 are now defined in global scope. And that can really cause a hard to find bug!
Another good reason for not doing this, is that extending the code may lead to developers adding code in between the var-declaration and the for-loop, for whatever reason. Then the variable declarations for the for-loop “drift away” and loose their context even a bit more. And the source code becomes harder to understand.
So we rather put all the variable declarations into the for-loop’s var-statement. Like so:
1 2 3 4 5 | var someVar = whatever, anotherVar = [1,4,5]; // This is outside the for-loop and also looks like it. for (var i=0, len=nodes.length, node; i<len; i++){ node = nodes[i]; // whatever } |
Now we have a pretty compact and well context-focused syntax for declaring variables needed within a for-loop. This code is nicely readable and has no impact on performance, even if in most cases good and maintainable code is worth more than the fastest code, but that’s a use case based decision! We are taking this as a standard for uxebu code.
What do you think? Let us know if you see potential for optimization.
Comments
Nice writeup. In qooxdoo we use the default way most of the time but tend to use the final version in performance critical code. I don’t use this kind of code everywhere because I feel its slightly less readable.
Regarding a code verifier: We already have a static code analysis tool in qooxdoo. Maybe we can join forces on this topic.
March 29, 2009 — 12:07 pm
Fabian Jakobs
I think in the direction of a coding style verifier for a first tiny step. It would be really interesting if we could join forces on this topic. The area of continuous integration, code verifying and all the development process with JavaScript is still very underdeveloped.
March 31, 2009 — 06:34 pm
Wolfram Kriesing
You forgot a few optimizations. If you are trying to reduce the number of bytes/chars in the script, you can combine the incrementing step with one other reference.
Example:
for (var i=0, len=nodes.length, node; i<len; ){
node = nodes[i++];
// whatever
}
In the case of cycling through nodes, you can use this form (#2):
for (var i=0, node; node = nodes[i]; i++){
// whatever
}
Bring both of these together:
for (var i=0, node; node = nodes[i++]; ){
// whatever
}
I’m aware the last two using ‘node = nodes[i]‘ will only work in certain situations like dom nodes or looping through other enumerable structures where you are guaranteed all values are non-false.
For the most part, I use what you arrived at as the final solution, but I also use my second example where appropriate. As a rule, I generally never move the incrementer parameter from its default position.
April 4, 2009 — 11:31 am
fearphage
Thanks fearphage, this is good input. Thank you! You are right, this would have been worth appearing in the article. But as you are mentioning, the last solution really requires a bit more brain-work to be understood and might not be “compatible” with all the team.
The solution we finally came up with and use at uxebu seems the most explicit to me. It doesn’t hide things and is still quite compact.
April 4, 2009 — 06:45 pm
Wolfram Kriesing
Why not put the ‘var node’ declaration inside the loop?
for (var i=0, len=nodes.length; i<len; i++){
var node = nodes[i];
// whatever
}
Is this not clearer than your final version? I think it reduces clutter, since to me the vars inside the for-loop declaration should only be related to the for-loop control flow
June 4, 2009 — 05:46 pm
Alex
@Alex the only reason is code size. It’s just 4 bytes, but that might add up over multiple occurences in the code, and actually it’s just a matter of getting used to. I can tell from experience :-)
June 5, 2009 — 12:27 am
Wolfram Kriesing