IDEA should find this programming bug

when I want to use the variable i as loop control variable, the IDEA tell me it has been used, so I use i2, but I copy and paste code fragment and edit in loop body, I forget to change the 'i' to i2, a terrible bug occurs:

for(int i: v) {
....
for(int i2: d) {
c+ = in.read()
}
....
}

the right should be "c[i2] = in.read();"
when the loop body is large, it is hard to find such error.
hope the IDEA can warn me for it! so it is really intelligent IDE

9 comments

sorry, the error pattern is :

for(int i: v) {
....
for(int i2: d) {
c+ = in.read() //it should be c[i2]
}
....
}

0

sorry, the error pattern is :

for(int i: v) {
....
for(int i2: d) {
c[ i ] = in.read() //it should be c[i2]
}
....
}

0

oh my gode, if I do not write c[ i ] as c[ i ], so it should be c missing the

0

Good one! Please submit a JIRA request. Call it something like "New Inspection: Loop variable not used in loop"

--Dave Griffith

0

Dave Griffith wrote:

Good one! Please submit a JIRA request. Call it something like "New Inspection: Loop variable not used in loop"

--Dave Griffith


???
Isn't this already caught by the regular "Unused local variable" inspection?
Do you want to have another inspection, so that you can enable/disable it independently?

Honestly, I made this kind of error myself. Though I don't really know how Idea should find it
automatically: The wicked thing is, that I do use the inner loop variable somewhere in the loop,
but at the one place where some loop object is initialized, I still erroneously use the outer
loop variable.

My workaround is to never ever use names like "i", not even in the smallest loop. It's much
harder to err on names like rowIndex and columnIndex (though I 'succeeded' there too,
occasionally).

0

>Isn't this already caught by the regular "Unused local variable" inspection?

For for-each loops, it would be caught. For old-style for loops, the variable might be used in the condition or update, but unused in the body. That seems to me to be a bug pattern worth catching (i.e., I would have triggered it more than once in the last three months).

+
Honestly, I made this kind of error myself. Though I don't really know how Idea should find it
automatically: The wicked thing is, that I do use the inner loop variable somewhere in the loop,
but at the one place where some loop object is initialized, I still erroneously use the outer
loop variable.+

Well, you could just not nest loops. I'm only half joking on that. There are coding styles that recommend refactoring each loop into it's own method, both for the reason you indicate and because it supposedly leads to greater clarity. In support of those styles, IG does have an inspection warning if you have more than one loop per method. The idea breaks down if you've got to do a lot of matrix manipulations, but for enterprise business coding it's actually not a bad heuristic.

--Dave Griffith




0

Dave Griffith wrote:
>> Isn't this already caught by the regular "Unused local variable" inspection?


For for-each loops, it would be caught. For old-style for loops, the variable might be used in the condition or update, but unused in the body. That seems to me to be a bug pattern worth catching (i.e., I would have triggered it more than once in the last three months).


Convinced me.

Well, you could just not nest loops. I'm only half joking on that. There are coding styles

> that recommend refactoring each loop into it's own method, both for the reason you indicate and
> because it supposedly leads to greater clarity. In support of those styles, IG does have an
> inspection warning if you have more than one loop per method. The idea breaks down if you've got
> to do a lot of matrix manipulations, but for enterprise business coding it's actually not a bad heuristic.

Also true, I think small, well-named methods are good style. Probably I have to try and use them
more in this case.
However it often won't work, even for business logic. What if you update a bunch of variables in
the inner loop (counters, sums, etc.). You'd have to create a class to hold all these results from
the inner loop. (Or make these variables fields, but then you open up to other problems with
multi-threading and memory leaks.)
In general I think in-out parameters and multiple return values add more complexity than they are worth,
but sometimes I just wish we had that in Java.

0

but if there are 10 i2 occurs in the loop body, but some error being written as 'i', how to inspect it by 'local variable unused'?
yes, i is good, but i2 is bad, do not use i2 with i in the same time.

0

Right, that's the problem that there's no real way to check for, since 'i' is completely valid at that point. There are so many common coding patterns that use the outer loop variable inside the inner loop that testing for that would result in too many false positives to be useful. How would you check for equality of two collections, or find the first instances of some set of element in a stream, if you couldnt' use an outer loop variable inside an inner loop.

--Dave Griffith

0

Please sign in to leave a comment.