Wednesday, July 19, 2006

Marick on refactoring

Brian Marick just posted his definition of refactoring: "A refactoring is a test-preserving transformation." It very succintly expresses the critical need for tests. If you don't have tests, how do you know refactoring preserves anything? Great stuff as usual from Brian M.

8 comments:

joseph knecht said...

'Test-preserving transformation' is not an adequate definition. It is at least possible that 1 day computers will be able to write or verify the correctness of programs that don't have bugs, or don't need tests, but in such a world, there would still be refactorings.

A refactoring is a "behavior-preserving transformation", but I'll grant you that we generally (today) determine that behavior is preserved via tests.

If it were really a 'test-preserving transformation', then an app that has 1 trivial test might be able to have 90% of its code deleted, and that would be 'test-preserving' and thus a refactoring, but it is not really a refactoring, because behavior has changed, whether your tests catch it or not.

Grig Gheorghiu said...

Joseph -- thanks for the comment. You're right about the scenario with 1 trivial test. However, when I (or Brian Marick to be exact :-) said "test preserving", the idea was that your application has a comprehensive suite of automated tests. Code coverage for your unit tests can and should be in the 90% range. That's when you *start* having some confidence in your refactorings. Of course high code coverage numbers are necessary but not sufficient -- but this is a different disussion ;-)

joseph knecht said...

Hi Grig, I see what what you mean, but refactoring as a concept makes sense even when you don't have any tests, so it can't be defined as test-preserving.

I believe a better definition is "behavior*-preserving", where behavior* is defined in terms of sufficient conditions and necessary conditions. One of the sufficient conditions could be all tests in a properly tested app still pass, but it wouldn't be a necessary condition, so shouldn't be essential to the definition.

Here's another scenario: a "refactoring" actually does not change the high-level behavior, and the app was well tested, but because of the way something was coded, the original tests did not catch a problem. After the refactoring, a test that passed no longer passes, but not because it wasn't a refactoring, but because the tests did not catch that particular problem because of the way the code and/or tests were written. This is not a far-out scenario, because even in a well-tested app, only a tiny portion of the entire state space is really tested.

Also, back to the original issue: if you or Brian really mean "test-preserving in an application that has a comprehensive suite of automated tests", then shouldn't your definition be: "test-preserving transformation of an application that has a comprehensive suite of automated tests", in which case the trivial scenario would not fit the definition.

Grig Gheorghiu said...

Joseph -- I agree with you that Brian Marick's definition isn't perfect, and doesn't apply to 100% of the scenarios that involve refactoring. I don't see that definition as a scholarly one, but more as a Zen koan, that shakes people a bit out of their complacency. In this sense, the terseness of the definition makes it much better than the variant you suggest.

joseph knecht said...

I'll buy that.

Michael Chermside said...

Yes, but I strongly agree with Joseph's claim that "behavior preserving transformation" is a better definition. Tests are only one way of measuring behavior.

A quick check of Fowler yields: "Refactoring is the process of changing a software system in such a way that it does not alter the external behavior of the code yet improves its internal structure." so we're clearly not the first to put this in terms of behavior.

I think you might have an interesting point if you restricted the preserved behavior to preserving "behavior we care about" -- so, for example, a transformation that altered what gets written to debugging log files would still be a refactoring. But it is a VERY rare project where "tested behavior" is the same as "behavior we care about".

Dave Nicolette said...

Let me cast my vote with Joseph and Michael on this one. When refactoring I think we need to be focused on preserving behavior. The test suite is only a tool, not the end in itself.

Sure, someone like Brian or Grig takes it for granted that there's a comprehensive test suite, but as we bring traditional colleagues along we don't want them thinking about green bars as more important than the actual behavior under test. Brian's definition might tend to lead to that sort of thinking.

Grig's interpretation of Brian's definition as a koan is interesting. My worry is that a young monk whose head is only freshly shaved won't quite get the gist of it. By their nature, koans don't have straightforward answers. People who are learning how to refactor need straightforward answers.

Grig Gheorghiu said...

Michael -- I guess my point is that although the official definition of refactoring is "a behavior-preserving transformation", that definition remains theoretical until you can back it up in practice. How do you back it up? By testing -- I don't see any other way. You can tell yourself all you want that you're preserving behavior, but you never know for sure until you have a large safety net of automated unit and functional tests.

I know the projects that have these automated test suites are VERY rare, but that's exactly the point of this new and apparently controversial definition: to raise people's awareness in this area.

Speaking of log files, you'd be surprised how important they are in certain types of testing. For example, TextTest is a "behavior-based" functional testing tool (note the word behavior) which bases itself *exclusively* on the log files/stdout/stderr of the AUT.

Dave -- I think people new to the realm of agile development and testing need to have their old notions shaken up a bit. Hence my comparison with the koan. If the young monks with fresh minds can't take in a fresh fact, then who can? People who have been used to debugging and eye-balling their software all their lives?