tags:

views:

149

answers:

3

Is it considered poor style / discourage to ignore the loop variable in a for-each statement in Java?

I have some code looks looks sort of like the following:

public void makeChecklist( final List<File> inputSrcs ){

   for( File src : inputSrcs ){
      System.out.print( src.getName() + "\t" );
   }
   System.out.println();


   for( File src : inputSrcs ){
      //Not using "src" in this body!
      System.out.print( "[ ]\t" );
   }
   System.out.println();

}

Is this a bad idea? Any reason not todo things this way? It just seems so much cleaner than using a regular for-loop.


PS- presume that for the example above I want the checkboxes to appear underneath the names, the example is contrived to ilustrate my question as simply as possible.

+8  A: 

It certainly looks odd. I would make it clearer that it's only the count that matters with a for loop:

for (int i = 0; i < inputSrcs.size(); i++) {
    System.out.println( "[ ]\t" );
}

I think that makes the intention clearer. Although as has been pointed out in the comments, we're really just replacing one "dummy" variable with another in the above. What I like about it is that it explicitly calls size(), which I believe shows that the size is important.

In a more expressive language you might find something to indicate "I just want to execute the body n times" which would be prettier still:

inputSrcs.size().times() {
    System.out.println( "[ ]\t" );
}

(That may or may not be valid Groovy :)

EDIT: Another obvious answer occurs to me, which should have occurred before:

printRepeatedly("[ ]\t", inputSrcs.size());

...

private static void printRepeatedly(String text, int count) {
    for (int i = 0; i < count; i++) {
        System.out.println(text);
    }
}

Now in the calling method, the meaning is absolutely obvious... and within printRepeatedly we don't even have the context of a list, so we couldn't possibly be trying to use the data. At this point the dummy variable i is fairly obviously a dummy variable, and the method name makes it obvious why we'd want this behaviour.

Jon Skeet
I rather disagree. I see the OP's code printing a checkbox for each file. The intention there is *crystal* clear, and adding a counter variable and all that just muddies the waters.
cHao
@cHao: Well, I know that *I* would be looking for the use of `src` in the second loop, frowning when I couldn't see it. (Assuming there wasn't the comment, of course.)
Jon Skeet
Well, you're replacing the looped variable that's being ignored with a counter that's being ignored - not sure if that's preferred.
zigdon
@zigdon: Indeed, which is why I prefer the second snippet in a language which allows it :) However, at least it explicitly calls `size()`, which is the important bit: the output depends on the size of the list, and *only* on the size.
Jon Skeet
@Jon: If the variable were called "unused" or "dummy" or "ignored" or the like, you'd have no reasonable reason to frown. :)
cHao
@cHao: Yes, that would be better, definitely.
Jon Skeet
There's a Chuck Norris/Jon Skeet joke in here somewhere, I think. :D
LBushkin
+2  A: 

I think if you're going to do it, the comment removes a lot of the ambiguity/potential confusion.

Brian Agnew
A: 

Seems perfectly clear to me.

You will find that the line feeds added by println()--as opposed to print()--will make it so your check boxes aren't really lining up with anything other than your left margin.

Matthew Flynn
True enough, I will correct the code in this question, but I didn't put much thought into it since it was contrived for the purpose of this question. :)
ArtB