We’ve decided to go back to reading a book in our technical book club after a few months of discussing different papers and the chosen book is Michael Feathers’ ‘Working Effectively With Legacy Code‘.
We started off by reading the first two chapters titled ‘Changing Software’ and ‘Working with Feedback’ and these are some of my thoughts and our discussion of the chapters:
- Early on Feathers talks about the need to change software in order to add features and fix bugs and while it is certainly necessary to make some changes to code in order to do this we discussed whether there is ever a time that we might look to keep the number of changes we’re making to a minimum.
Tom suggested that if we have good enough tests then we shouldn’t be fearful of making changes at any time. I think we’d look to be more careful about making changes around the time of a release because we don’t have a lot of time to recover if we make a mistake. Perhaps that only suggests that we don’t have good enough tests though!
- Something which I’ve noticed recently is that we often end up with transitionary refactorings in our code base which are attempts at refactorings which haven’t quite been completed yet. I think this is somewhat inevitable if we are making incremental changes to improve the quality of the code base.
The problem with this is that it is sometimes it is not obvious where that refactoring is going and if someone other than the original authors has to work with the code then they can easily drive it in a different direction.
While we were discussing this it reminded me of an idea we read in ‘An agile approach to a legacy system‘. The authors suggest that whenever there is a big refactoring to make the whole team only works on that until it is completed. It would be interesting to see how well this approach would work with a bigger team.
- I really like the definition of unit tests that Feathers uses – tests that give us ‘localised feedback and ‘run fast‘. I wrote a post last year where I tried to break this down further but I think Feathers’ guideline is useful to keep in mind when writing these tests.
It’s easy to end up relying on functional tests which undoubtably have their place but don’t provide the rapid feedback that we need to work effectively.
- We also discussed the need to think twice before creating a technical debt card or adding a ‘TODO’ comment to a piece of code. More often than not these just end up being ignored so it makes sense to check whether you can make the change when you see the problem where possible.
I find TODO comments a symptom of this – a colleague of mine once said that he was annoyed by TODOs, that they’re a statement of ‘I can’t be bothered doing this, I want YOU TODO this”.
It’s certainly not always possible to fix things immediately but my current thinking is it probably makes sense to note that down somewhere that’s not in the code and get back to it as soon as possible. Having said that I was reading Mario Fusco’s entry in ‘97 things every programmer should know‘ earlier and he recommends putting ‘TODO’ comments into the code base to identify areas of code to come back to later. Perhaps it just depends on the team.
- I think the following observation is quite astute:
Breaking down a big class into pieces can be pretty involved work unless you do it a couple of times a week. When you do, it becomes routine. You get better at figuring out what can break and what can’t, and it is much easier to do.
In addition if we don’t do this type of refactoring then those classes increase in size and it becomes even more difficult to do in future.
Tom also pointed out that the more we practice the better we become at identifying good and bad code which allows us to better focus our attention on where we need to make improvements.