2

Below code is resulting in "Avoid declaring or assigning variables in a loop that are not dependent on the loop condition". (According to the coding best practices)

 private void testingLoop() {
    String var[]= {"java", "code", "review"};
    String arr[] = new String[1];
    for(String i : var)
    {
    arr[0] = i.concat("Script");
    }  
    System.out.println("The result is: " +arr[0]);
}

Why is it considered as a bad practice when we assign variables inside a loop ? any solution to overcome this problem ?

Note: My intention was only to show the for loop, so don't consider much about the purpose of the code.

HookUp
  • 393
  • 1
  • 6
  • 20
  • Sometimes you may be tempted to argue in the comments, and then you remember that no one has ever changed anyone's mind in the comments. And then you start name calling because you've run out of things to argue about. And then you realize, these words are all ephemeral and don't exist physically. And then you realize your entire job consists of moving non-physical bits around. And then you realize there's a whole cottage industry built up around solving problems that don't actually exist in the real world. And then you're sad. Don't be sad. Don't argue. – George Stocker Jan 28 '16 at 21:42

4 Answers4

1

Note that The scope of local variables should always be the smallest possible.

From a maintenance perspective, declaring or assigning variables in a loop is better than the other approach. Declare and initialize variables in the same place, in the narrowest scope possible. Don't leave a gaping hole between the declaration and the initialization, and don't pollute namespaces you don't need to.

For more referance: Declaring variables inside or outside of a loop

Community
  • 1
  • 1
GroundIns
  • 541
  • 1
  • 5
  • 11
  • I have gone through that, I understand the reason why declaring variables outside the loop is better , but here I am assigning. – HookUp Jan 28 '16 at 17:45
  • what do you mean by 'but here I am assigning' ? – GroundIns Jan 28 '16 at 17:46
  • @HookUp *I understand the reason why declaring variables outside the loop is better*: this answer says exactly the inverse of what you understand, and it's correct. Declaring the variable inside the loop is better, because the variable has a smaller scope: that makes the code easier to refactor, and leaves less room for bugs. – JB Nizet Jan 28 '16 at 18:35
  • @JBNizet: Yes you are correct ! But I assume that it depends on whether you are using the variable outside the loop or just inside the loop. – HookUp Jan 28 '16 at 22:01
1

I'm not too sure myself but I can try to have a crack at it.

Your code is currently replacing the value of arr array. So the first time around the loop, it stores javaScript, and second time around, codeScript, and last time around reviewScript. My only problem with this is that when you're printing it at the very end, it's only going to print reviewScript each time because that's the last String value that your for each loop got.

This could've just as easily been:

private void testingLoop() {
    String var[]= {"java", "code", "review"};

    // No need for this anymore

    // String arr[] = new String[1];
    // for(String i : var)
    // {
    //     arr[0] = i.concat("Script");
    // }  
    // System.out.println("The result is: " +arr[0]);

    int lastIndex = var.length;
    System.out.println("The result is: " + var[lastIndex - 1] + "Script");
}

So I think the reason why it's giving the message is because there's no reason for you to assign anything inside the for loop; only to get the last element in you var array. You can concat at the end AFTER finding out the index of the last element in var (although that could be done through var.length like I mentioned before).

sparkhee93
  • 1,381
  • 3
  • 21
  • 30
0

Try:

private void testingLoop() {
    String var[]= {"java", "code", "review"};
    List<String> arr = new ArrayList<String>();
    for(String i : var)
    {
        arr.add(i.concat("Script"));
    }  
    System.out.println("The result is: " +arr[2]);
}
matt2000
  • 1,064
  • 11
  • 17
  • I don't want to change it to List because later in my code I am sending it to a method which expects a String. – HookUp Jan 28 '16 at 17:42
0

I suppose the warning means that if you can declare / assign the variable outside the loop with the same results - then it's better to do it 1 time than N times.

private void testingLoop() {
    String var[]= {"java", "code", "review"};
    String arr[] = new String[1];
    for(String i : var)
    {
    arr[0] = i.concat("Script");
    }  
    System.out.println("The result is: " +arr[0]);
}

Is the same as

private void testingLoop() {
    String var[]= {"java", "code", "review"};
    String arr[] = new String[1];
    arr[0] = var[var.length - 1].concat("Script");
    System.out.println("The result is: " +arr[0]);
}
Daniel
  • 6,194
  • 7
  • 33
  • 59