Code review


I'm reviewing a large body of code for a client at present. It's a general review of the design, coding style, code correctness and testability of a project. I started off by making notes on some of the general design changes that I'd recommend and eventually got down to a detailed review of the code. There's a lot to say about the code and, as with all code, sometimes it's hard to figure out the reason behind the use of a particular construct. I was starting to think that the note taking was going to take an age when I realised that there was a much more concise way to record my thoughts on what was wrong with the code. I changed it.

Of course, since the client is using a heavy-weight source control system, changing the code to illustrate what's wrong with it isn't in any way dangerous. We don't intend to check-in any of the changes I'm making, but I do make sure that they all compile. I'm using the compiler to chase down the coupling; I change X to a more suitable construct and watch as Y and Z complain about it, then I look at Y and Z and can often see what the unusual construct at X was trying to achieve... Of course I still need to take some notes, but I found that this way is much faster as I only need to comment on the strangeness, rather than commenting on everything... When I need to write up the report I simply diff my changes against the original sources and write about the implications of not applying the code changes. Seems to work...

Leave a comment