Tuesday, May 11, 2010

Automated refactorings... check that they work!

Many modern IDEs provide automatic, context-sensitive refactorings, especially for statically-typed, popular languages like Java. While you should use these wherever it saves time, you should check that they preserve semantics on the code they transform.

For example, in the following test code, Eclipse will offer to "exchange left and right operands for infix expression" when you try a quick fix (cmd-1 on a Mac, ctrl-1 elsewhere) at the '&&' on line 2. The refactoring transforms "(a || b) && c" into "c && a || b".

 boolean a = true, b = true, c = false;
boolean original = (a || b) && c;
boolean swapped = c && a || b;
assertTrue(!original); // ok
assertEquals(original, swapped); // fail

Note that the parentheses have disappeared - at first I assumed that they must therefore be unnecessary and "c && a || b" is therefore equivalent to "c && (a || b)", but figured that the parser should evaluate from left to right (which would produce different results). Being unconvinced one way or the other, I made a simple testcase to verify that both expressions were logically equivalent, which failed.

I'm not sure why Eclipse removed the parentheses, and it doesn't really matter - the point is, keep an eye on what your code transformation utility is doing - don't assume that it must produce correct output. This was one of those cases where the logic bug might not show up for a while, and the existing unit tests wouldn't have caught it, unless I bothered to run a (probably outdated and slow) integration test.

No comments:

Post a Comment