views:

2051

answers:

7

JSLint (with the onevar flag turned on) is flagging some javascript code that I have with the following:

Problem at line 5 character 15: Too many var statements.

I am happy to fix these errors, but I'd like to know, am I doing it for performance or because it is just a bad practice and has a greater potential to introduce bugs in my javascript code. What is the reason behind the onevar flag?

I did look at the JSLint docs for the var keyword but it doesn't specifically talk about why multiple var statements in the same function are bad.

Here is an attempt at an example. Explain how the code will benefit from only having 1 var statement:

function Test(arg) {
   var x = arg + 1,
       y = cache.GetItem('xyz');
   if (y !== null) {
      // This is what would cause the warning in JSLint
      var request = ajaxPost(/* Parameters here */);

   }
}
A: 

Just a guess here, but it may be time for functional decomposition. Functions should do one thing and do it well.

Too many vars is suggestive of a function that's trying to do too much. Or a case where you should be using an array.

tpdi
A: 

If the "onevar" option is set to true if only one var statement per function is allowed.

if (funct['(onevar)'] && option.onevar) {
    warning("Too many var statements.");
}
tom
Okay, I clarified in the question that I specifically turned that on. So I know why it is warning me, but what does fixing those warnings do for my code?
slolife
+31  A: 

Javascript does not have block scope. In other languages with it (like c), if you declare a variable in the if statement, you can not access it outside of it, but in javascript you can. The author of JSLint believes it is a bad practice, since you (or other readers) might get confused and think that you can no longer access the variable, but you actually can. Therefore, you should declare all your variables at the top of the funcion.

swampsjohn
Better answer than mine, modding up.
tpdi
A: 

The idea is that you should use an object instead of individual vars. So where you have got:

var x = arg + 1,
    y = cache.GetItem('xyz');

Change it to:

var dimensions = {};
dimensions.x = arg + 1;
dimensons.y = cache.GetItem('xyz');
dimensions.request = ...

You can then access these variables through the object, its neater to have one object per function to contain that functions variables. Then you won't get the warning.

+1  A: 

Just declare your vars in one place like this:

var request,x,y;

Roy Walter
+4  A: 

The official reason is here, by Douglas Crockford.

Elzo Valugi