Today while reviewing some code* I came across the following:
// TODO: Refactor this.
Followed by a lot of code which clearly needed some love and attention. Why is this code smell? Well it’s undone work just for starters. Someone checked this in and called it good, knowing that it was incomplete.
Then you’ve got to ask yourself… Why this was left as a “TODO”? This is the real issue.
What the code really needed was a series of simple refactorings based around extract method, followed by some general moving of things around. Not exactly a gargantuan task. But in order to refactor code quickly and safely you need some tests around it. In this case there were no tests at all!
This is of course a “WTF!” moment. Not only is the code itself incomplete but the work is incomplete because it’s untested.
The lesson. If you’re reviewing code and you see a “TODO: Refactor this…” ask yourself, do we have a deeper issue here?
* I’m happy to say this was not source code delivered as guidance by p&p.