Friday, June 8, 2007

From iterator-loop to foreach-loop with one regex.

Probably the most important feature we've worked on this release was to add Java 5.0 support to EMF. We've learned a lot during the process and actually had some fun trying to make sense out of Generics and Annotations. Of course there were also some very annoying tasks, such as converting Java 1.4 "Iterator loops" into the new "foreach loop" style. For example, this snippet

for (Iterator i = Collections.singleton(new Integer(1)).iterator(); i.hasNext();)
{
Integer integer = (Integer)i.next();
System.out.println(integer);
}

should look like this after the modifications
for (Integer integer : Collections.singleton(new Integer(1)))
{
System.out.println(integer);
}

Unfortunately Eclipse's "Source->Clean..." magic action doesn't do a good job here since it doesn't keep the names and types of the "each" variable. At least back in December, I would end up with Object object : instead of Integer integer : for the example above.

Either to boost my productivity or just to exercise the right to be lazy ;-), I've come up with a regular expression that does the conversion for me. It works flawless with Eclipse's Find dialog (ctrl+f):



Here's a clipboard friendly version:
Find:
(for\s*\(\s*)Iterator\s*\w*\s*=(.*)\.iterator\(\).*;\s*(\)\s*\{\s*)(.*)\s*=.*next\(\);\s*

Replace with:
$1$4:$2$3

5 comments:

Karl Matthias said...

Hah, very nice! Gotta love regex. :)

George said...

Great. Works fine in most of the cases, but could lead to hard to find bugs as well (so you can't use Replace All):

boolean found = false;
for (Iterator i = Collections.singleton(new Integer(1)).iterator(); i.hasNext() && !found;) {
final Integer integer = (Integer) i.next();
System.out.println(integer);
// ...
}

or mess up commented out code:

// for (Iterator i = Collections.singleton(new Integer(1)).iterator(); i.hasNext();) {
// final Integer integer = (Integer) i.next();
// System.out.println(integer);
// }

or create garbage:

for (Iterator i1 = Collections.singleton(new Integer(1)).iterator(), i2 = Collections.singleton(new Integer(2)).iterator(); i1.hasNext() && i2.hasNext();) {
final Integer integer = (Integer) i1.next();
System.out.println(integer);
}

or doesn't work:

for (Iterator i = Collections.singleton(new Integer(1)).iterator();
i.hasNext();) {
final Integer integer = (Integer) i.next();
System.out.println(integer);
}

Marcelo Paternostro said...

George: WOW... I am glad that 80% of my loops, including the ones in the JET templates, were as simple as it can get ;-)

Benno Baumgartner said...

The iterable loop converter requires, that the Iterator is not defined with a raw type. It works if you write:

for (Iterator<Integer> i = Collections.singleton(new Integer(1)).iterator(); i.hasNext();)
{
Integer integer = (Integer)i.next();
System.out.println(integer);
}

instead.

We are conservative with the loop converter because as george pointed out, converting a loop can (if not done correct) infer semantic changes which are very hard to detect.

See:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=194638
https://bugs.eclipse.org/bugs/show_bug.cgi?id=194639

P.S. you can always file a bug report if a feature does not do what you want;-)

Tom said...

And that's it not missed "new Integer(2)" is very bad you should:
a) Use autoboxing
b) Integer.valueOf(1)