One of the definitions of refactoring is to change the structure of the code without changing its behaviour. This requires the existence of unit tests. However…
Sometimes we want to both change the structure AND the output of our code… and sometimes there aren’t even any tests!!!
The seemingly easy way of doing this is:
- Write some tests for how the code should behave
- Change the code until those tests pass
This is easy to do, but quite hard to review in a code review. There’s a big unreviewable change, which was, by definition, made without the original tests as a reference. It’s not a refactor – it’s just a smudge in the codebase.
As has been discussed before, code reviewability is a huge success factor for software development teams.
An Alternative Approach
I tried this the other day and it worked for me.
Start with characterising tests that black box test the current implementation – these will accurately show the behaviour you want to change, however because they’re fresh test code you can write them both to explore the current implementation AND highlight the particular behaviours that are wrong now and will change later.
Next we get those tests reviewed and merged. The team is being asked to look at just a, hopefully, nice test fixture. It should be an easy review.
Then, modify the tests to describe the desired outcome.
Then modify the code to achieve that outcome.
When I did this, it was a fairly self-contained method that calculated a single string, so I modified all relevant outcomes and reworked the code to make them all pass. If possible doing this incrementally, perhaps in multiple pull requests and merges would be better. Dependent on size.
When I reached my next PR, the team was being asked to see how the behaviour had change – this was largely just a matter of observing slightly different expected test output, and then seeing the rework of the code in the context of passing tests.
It worked well when I tried it.
Published on Java Code Geeks with permission by Ashley Frieze, partner at our JCG program. See the original article here: Refactoring and Changing Opinions expressed by Java Code Geeks contributors are their own. |
