Interesting Groovy Loops Detail Tuesday, August 07, 2007

While fixing a bug in Grails last night I came across something that I found interesting. At first I was really puzzled but now that I know what is going on, it all makes perfect sense.

A unit test that had been passing for some time just started failing. I isolated a specific revision of a specific file that seemed to be causing the test to fail. The test passes with version X of the file and the test fails with version X+1 of that file. I spent some time diffing the 2 revisions and couldn't find anything that seemed relevant. Eventually I figured out the specific change that was causing the failure. There was code that looked something like this...


someCollection.each { someElement ->
// do something with someElement
}


That code had been changed to look something like this...


for(someElement in someCollection) {
// do something with someElement
}


The code inside the loop was exactly the same for both versions. The only difference is the original code was using "each" to iterate over the collection and the new code was using a "for" loop to do the same thing. I puzzled over that for a bit trying to figure out what was going wrong and after discussing with my co-developers the problem eventually became clear. Code inside the loop was making use of someElement inside of a closure and later that closure was being executed. Consider this...


def listOfClosures = []
def numbers = [1, 2, 3]
numbers.each { number ->
listOfClosures[number] = { println "number is $number" }
}
listOfClosures[1]()
listOfClosures[2]()
listOfClosures[3]()


As you might guess, the output from that looks like this...


number is 1
number is 2
number is 3


Now consider this...


def listOfClosures = []
def numbers = [1, 2, 3]
for (number in numbers) {
listOfClosures[number] = { println "number is $number" }
}
listOfClosures[1]()
listOfClosures[2]()
listOfClosures[3]()


The output from that looks like this...


number is 3
number is 3
number is 3


With the "for" loop approach, the same "number" variable is used each time through the loop so by the time the loop completes, the value of "number" is 3 which is what is used by the time the closures execute. With the "each" approach, a separate variable is used for each iteration so each closure accesses the "number" that was around when the closure was declared. None of that is broken or a bug. I believe it is all working as it should.

Thanks to Charles Nutter and Graeme Rocher for helping with that.

3 comments:

Chris Dolan said...

The key is that your for loop is not making a lexical iterator. In Perl, I can make it work either way depending on whether I make the iterator lexical with the "my" keyword:

@numbers = (1,2,3);
%list_of_closures = ();
for my $number (@numbers) {
$list_of_closures{$number} = sub { print "number is $number\n" };
}
$list_of_closures{1}->();
$list_of_closures{2}->();
$list_of_closures{3}->();

yields

number is 1
number is 2
number is 3

But if I remove the "my" from the loop variable, like so:

for $number (@numbers) {

I get

number is
number is
number is

because the variable is now shared in the closures instead of lexical per closure, and the value is null after the map invocation is over.

jfiore said...
This comment has been removed by the author.
Jeff Brown said...

Fiore,

I had a great time last night at the Atlanta JUG. Thanks for having me.

I didn't say that there are any issues with Groovy's exception handling. What I did say is that in Groovy you are never forced to catch exceptions. For the scenarios where you do want to catch exceptions you can put your code inside of a try/catch block just like you can in Java. The difference is in Groovy you never have to do that, even if you are invoking a method that declares that it throws a checked exception.

I hope that all makes sense. I will drop you a note with my contact info.