3

Please bear with me, I could only explain the problem in the comments of the code snippet below. Please read through them carefully. Question at the end:

I usually tend to name the variables used in for loops with i, like this:

 for (var i = 0; i < arr.length; i++){
      // do some COMPLEX operations here
 }

This looks fairly simple; but when I'm at it I may require nested for loops within the above for loop. So, introducing the complexity now:

 for (var i = 0; i < arr.length; i++) {
      // do some
      // VERY
      // COMPLEX
      // operations
      // here

      for (var i = 0; i < javascript_object.list.length; i++) {
          // I may have inadvertently forgot of the variable 'i' was been used earlier
          for (var j = 0; j < javascript_object.list[i].another_list.length; j++) {
              // Perform more complex operations here
          }
      }

      this.getReports(arr[i]) // the value of this i
         // which I now think refer to the first for loop might have been changed
         // in second for loop and may lead to dangerous results! 
 }

How to avoid this from happening? How could the variables be named carefully so that it wouldn't clash with other variables being declared elsewhere?

10

One common approach, which is also used in the code you posted is to simply to use the next letter in the alphabet, so:

for(var i = 0; i < length; i++) {
    for(var j = 0; j < length; j++) {
        for(var k = 0; k < length; k++) {
            // More loops?
        }
    }
}

This approach is lazy, but it works. However, I find that it can get confusing when you're not looping the same array or accessing a multi-dimensional array. In that case, a better approach might be to simply provide more meaningful names:

for(var nameIndex = 0; nameIndex < names.length; nameIndex++) {
    for(var classroomIndex = 0; classroomIndex < classrooms.length; classroomIndex++) {
        // And so forth
    }
}

Of course this may be a little wordy for you, but I figure better a little wordy than completely uncomprehensible code. You were probably hoping for a neat hack here to be able to keep the i variable in its own scope. There may even exist a way to do so, however I don't believe in that kind of solution to a problem like this for two reasons:

  • Nested loops should generally be discouraged anyway. You cannot apply a best practice in a situation in which you're not following a best practice. Call a method instead.
  • It would leave the next programmer scratching his head wondering why you did it that way, and it is difficult to justify such a decrease in readability just to account for name clashing.

I hope that helps!

  • 2
    "Nested loops should generally be discouraged anyway" - so generally you would not iterate through a 2d or 3d array with a nested loop? Sometimes it makes sense to extract code into a method, sometimes it doesn't. – Benjamin Gruenbaum Sep 23 '13 at 12:44
  • 2
    @BenjaminGruenbaum Hence the "generally" in "Nested loops should generally be discouraged anyway." – Neil Sep 23 '13 at 12:46
  • I suppose choosing some meaningful names for the variables makes a lot of sense irrespective of where I use it. Learnt it the hard way though :) – Vineeth Sep 24 '13 at 3:16
  • 1
    @Vineeth General rule of thumb though: If you're having to deal with name clashes in loop variables, good indication to start dividing that function into bite-sized pieces. :) – Neil Sep 24 '13 at 7:44
14

Simple:

  // This block with a
  // VERY
  // COMPLEX
  // operation
  // here
  // is far too large.

When blocks become so large that you cannot see all of your local variable definitions any more, they are definitely too large. Refactor such a parts to a function, this will help a lot to avoid accidentally double declarations.

1

In your example the value of i will definitely be using the one that is in the second for-loop. This is because variables are function scoped in JavaScript. So it doesn't matter how many times you var it in a function, it will be the same variable.

To avoid this, either give the variables new names or enclose them in other function calls. Since you haven't specified what these "complex" operations are, I can't suggest any good refactoring method other than extracting the second for loop into it's own function.

  • I cannot post the entire code here since I will have to adhere to my company's security policies. Refactoring is something I will have to take care but I hadn't done it when I thought of this question in my mind :) – Vineeth Sep 24 '13 at 3:13
1

Use JSLint or JSHint to lint your code directly inside your IDE / Editor. From the Book "JavaScript: the Good Parts" by Douglas Crockford (author of JSLint):

JSLint is a code quality tool for JavaScript. It takes a source text and scans it. If it finds a problem, it returns a message describing the problem and an approximate location within the source. The problem is not necessarily a syntax error, although it often is. JSLint looks at some style conventions as well as structural problems. It does not prove that your program is correct. It just provides another set of eyes to help spot problems.

  • 1
    without an explanation, this answer may become useless in case if someone else posts an opposite opinion. For example, if someone posts a claim like "don't use JSLint nor JSHint, don't lint your code directly inside your IDE / Editor.", how would this answer help reader to pick of two opposing opinions? Consider editing it into a better shape, to fit How to Answer guidelines. – gnat Jan 14 '14 at 11:09
  • 1
    @gnat Thought it was self explanatory. Well, here you go - an explanation. – borisdiakur Jan 14 '14 at 11:25
1

If you used a transpiler and used the keyword "let" instead of "var", you would get an error at the second declaration.

for (let i = 0; i < condition1; ++i) {

    // SyntaxError: Identifier 'i' has already been declared
    for (let i = 0; i < condition2; ++i) {  
    }
}

but as others pointed out, https://martinfowler.com/bliki/FunctionLength.html, a function length more than 8 to 10 lines is probably too long.

protected by gnat Mar 11 '18 at 14:52

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.