One var statement for one variable!

April 2nd, 2010
wolfram

JavaScript allows to comma-separate multiple variable declarations, like so:

1
var i=0, j=1

. Declaring multiple variables using one var-statement accross multiple lines is a NO GO! I consider this evil. And I learned it the hard way. It might look pretty nice, looks like less code and more efficient. But it definitely is not so when writing code. If it results in more speed let your build tool, compressor or compiler do it. But don’t write code which spreads multiple variable declarations in var-statement across multiple lines!
PERIOD.

This article is quite long for such a simple topic, but anyways I just feel like writing it down, since this is how I initially used my previous blog and I must say it was very helpful for me to look up things again but obviously also for others to find information. I also feel that showing what coding guide lines to use and the reasoning behind it is really important especially also for those who have to apply them.

Good and Bad

Let me just quickly show examples for DOs and DON’Ts.

1
2
3
4
// DONT
var x = 1,
y = 2,
z = 3;

The above looks pretty slick. But not always! Especially when working in a team and when you will rewrite or touch the later code again, which basically is the case with all the useful code out there. The second and the third line depend on the first line. That means if a coworker comes along and pastes in his variable from his awesome and dead-simple code

1
2
3
// DONT
var y0 = 1,
y1 = 2;

into my code above. We might get the following:

1
2
3
4
5
// DONT
var x = 1,
y = 2,
y1 = 2;
z = 3;

And ouch, we have a global variable “z”. Of course he should change the trailing semicolon to a comma when pasting in his code. But his IM beeped, phone rang, coffee was done or he simply felt more like skiing and stopped hacking – and he forgot :). You know what I mean!
If the code hadn’t contained multiline variable declarations it wouldn’t have happened, like here:

1
2
3
4
5
// DO
var x = 1;
var y = 2;
var y1 = 2;
var z = 3;

Though there are cases when I still think that listing multiple variable declarations is ok, like the following. But just not on multiple lines!

1
2
3
// DO
var x = 1, y = 2, z = 3;
for (var i=0, l=arr.length, k; i

Real Life Examples

Let me show you three examples where I really felt the pain of the described problem, which finally triggered me to write this article. Those are real life examples, exactly the code I experienced the problem with. And at some point it was just enough.
The first example shows that a pretty complex piece of code is just not as easy to validate by humans. There is no bug in it, it is just hard to read code.

1
2
3
4
5
6
7
8
// DONT
var g = this.getObject("group.name", test) || groupName,
// Trim and replace all special chars, to have a proper test ID.
group = g.replace(/^s*/, "").replace(/s*$/, "")
.replace(/[^0-9a-zA-Z]/g, "_"),
// Replace multiple underscores with just one.
ret = ("ID_" + group).replace(/_+/g, "_")
.replace(/[_]+$/, "");

You can see on line 3 no comma is needed, because I just split up the chained calls for better readability. This may mislead the one who wants to understand the code. On the next line though a comma has to end the line because it is the end of the variable declaration. Additionally the code above has a comment after each variable declaration, which makes it easy to read and can be understood faster. If that was not the case it would be much harder to read the code. Just remember the one reading the code doesn’t want to read every line carefully, this is just very very time consuming. Therefore following certains rules can help to make this easier.
Another issue this code shows very well is that you first have to scan the code and understand the indentions, this sometimes means reading all the way up to the first variable declaration. It’s just unnecessary brain work.
So let’s make the source code easier to read, use a var statement for every variable:

1
2
3
4
5
6
7
8
// DO
var g = this.getObject("group.name", test) || groupName;
// Trim and replace all special chars, to have a proper test ID.
var group = g.replace(/^s*/, "").replace(/s*$/, "")
.replace(/[^0-9a-zA-Z]/g, "_");
// Replace multiple underscores with just one.
var ret = ("ID_" + group).replace(/_+/g, "_")
.replace(/[_]+$/, "");

This also makes understanding the indentions much easier. Doesn’t it? I just looked at the draft of this blog article and thought that the “wrong” source code above was wrong formatted because I didn’t understand the indention just by looking at the code (without thinking too much). Looking at the “fixed” source made my brain not start to think, it just looked all very clear to me.

In another real life example it took me three hours (three fucking hours) to find the bug. It was the following code in the dojo-mobile build script you can find on github. And I was debugging the code, reading it over and over again. I just didn’t find the bug. Try the following, scan the code quickly just once and read on.

1
2
3
4
5
6
// DONT
var parts = feature.split("-");
ns = parts[0], // The namespace of the feature, like "oo" in "oo" or "oo-declare.
f = parts.length>1 ? parts[1] : null, // The exact feature if given ("oo-declare").
ret = [],
data = globals.profileData;

Did you see it on first sight? Scan the code again. Believe me I read the code over and over again, I even thought there might be a variable declaration issue. But since my return variables are always named “ret” and my code was heavily asynchronously programmed I didn’t find out which declaration was the faulty one. It is fixed now (on github)!

1
2
3
4
5
6
// DO
var parts = feature.split("-");
var ns = parts[0]; // The namespace of the feature, like "oo" in "oo" or "oo-declare.
var f = parts.length>1 ? parts[1] : null; // The exact feature if given ("oo-declare").
var ret = [];
var data = globals.profileData;

Yet another example. This one actually just proved again that I definitely have to forget about the old way. You can also find it on github and feel free to follow the path of commits. My learning curve, if you will :).

1
2
3
4
// DONT
var depsData = _depsDataStack.pop(),
dir = depsData.directory;
file = depsData.file;

This one was not so hard to find, I agree. But still it was a bastard and stole my time. By sticking to a better convention I could have avoided it. Lesson learned.

Closing Thoughts

Just when preparing this article (which was supposed to be much shorter) I read through Douglas Crockford’s JSLint rules and funnily I found this one in there:

It is recommended that a single var statement be used per function.

I can say I disagree and I guess I gave some reasoning.

Though on the other hand I found this on the Crockford pages :)

It is preferred that each variable be given its own line and comment. They should be listed in alphabetical order.

Performance-wise, as stated above, I would leave the task to a compiler or alike tool to concatenate multiple variable statements into one, if it has an effect at all. The productivity and happiness of the programmers and not to loose time which could have been saved by just sticking to a simple convention is worth a lot more than pre-optimized code.
I am looking forward to feedback.

Comments

I agree that your 1st “real world” example is horrible form. I use a single var statement, but how I do it is like the following:

var a, b, b2, c;
a = 1;
b = ‘foo’;
b2 = b;
c = ‘bar’;

This satisfies Crockford and should also satisfy you. :-)

1

April 2, 2010 — 07:04 pm
Andrew Hedges

Crockford says one var statement per function so that they are all listed in one spot, and easily found. Also, while it is only a small gain, using comma separated values in a single var listings will save bytes when minifying your code.

Other than that, great job. Nice to have a different look on how to declare variable.

2

April 2, 2010 — 07:17 pm
Corey Hart

I never thought much about this issue, because I never declare several variables on one line (with the exception of for-loops, just like in your example above). So it’s good to know that I should not change this and start declaring multiple variables in one line. Thanks for saving me from hunting bugs that occur only because of a misplaced semicolon. :-)

3

April 2, 2010 — 09:50 pm
Stefan Kolb

This is a valid point. However, when your teammate runs JSLint, it will warn about the accidental global `z`. And if your teammate doesn’t run JSLint, then the build process will :)

And true, we should never try to do a job that a minifier will do better.

4

April 4, 2010 — 10:07 am
Stoyan

Totally agree, and can add couple of a bit different reasons:

1) I prefer to keep variables declarations closer to actual code they need for.

2) I refactor my code often. For me its very important to be able to move *any* part of the code to another function in the file or to another file at all. Obviously if I would bother with cherry-picking needed vars, that would take much longer time.

5

October 25, 2010 — 09:19 am
Nickolay Platonov

Your examples don’t give enough information to conclude if that’s the best way to do things. I personally don’t make a variable unless I use the value more than 1 time otherwise it’s just a waste. So in your first Real Life Example, that could potentially be combined into 1 variable delcaration. If the next line after those variables was `return ret` then there should be 0 variable declarations and you should just return the expression.

Example:

return (“ID_” + (this.getObject(“group.name”, test) || groupName).replace(/^s*/, “”).replace(/s*$/, “”).replace(/[^0-9a-zA-Z]/g, “_”)).replace(/_+/g, “_”).replace(/[_]+$/, “”);

You could pretty that up and optimize the regular expressions but yea. I think it’s a bad practice to make one-time use variables overall.

6

October 25, 2010 — 03:19 pm
fearphage

Woops. Submitted prematurely.

I personally use this format

var
foo = 1
,bar = 2
,catpants = {
cat: true
,pants: true
}
,$ = function() {
}
;

You don’t run into issues with “should this line end with a comma?” because none of them do. Also since the comma is on the line that requires it, I can remove a variable declaration without needing to edit anything else unless it’s the first variable declaration.

7

October 25, 2010 — 03:25 pm
fearphage

@fearphage I dont really agree. Esp. when you have code that complex, which regexps are unfortunately by nature, you better document them. And in this case maintainability, which means others shall have an easy time understand the code, is definitely much more worth than a small optimization like that.
And the minifier/compiler can do this optimization for me anyway. So I can’t agree.

And regarding your later example: that is very well possible too, just do never copy/paste the “foo = 1″ line, that will break the code too.

Thanks for the input! Very welcome.

Wolfram

8

October 25, 2010 — 05:00 pm
Wolfram Kriesing

Regular expressions are not difficult to comprehend at all… especially the simplistic examples in this post. They are very straight-forward and clear in my opinion. I’m aware that they can be much more complex however. Minifiers never remove veriables in my experience. So if you make a 1-time-use variable, it will have it’s name shortened but the minifier will not remove the variable and replace it with inline code… at least I’ve never seen such. If you have, please link me to this minifier.

9

November 8, 2010 — 05:16 am
fearphage

10

May 8, 2011 — 05:21 am
frenky

Just found another reason why multiple var statements are needed. If you try to debug:
var x = f(),
y = f1(),
z = f2();
you can NOT set a breakpoint on the f1() call.
var x = f();
var y = f1();
var z = f2();
doing this you can!
You can even do:
var x = f();
debugger;
var y = f1();
var z = f2();
just hit me and cost my precious time :(

11

June 9, 2011 — 03:41 pm
Wolfram Kriesing

You have a mistake in your first real-world example. Did you do that on purpose to make a point?
You wrote:
// DO
var g = this.getObject(“group.name”, test) || groupName,
// Trim and replace all special chars, to have a proper test ID.
var group = g.replace(/^s*/, “”).replace(/s*$/, “”)
.replace(/[^0-9a-zA-Z]/g, “_”);

That code has a comma followed by var.

12

August 25, 2011 — 05:56 pm
Sean

Thank you Sean, yes that is a typo. Fixed

13

August 25, 2011 — 08:14 pm
wkriesing

Leave a Reply