Code reviews or "how did I miss that?"
Load testing aside, it's nearly impossible to account for real live usage when writing test plans and some users have incredible talent for coming up with creative ways of using (or rather misusing) a program in a way it was never intended to function.
We just went through some of that with our new project at work and I think the common conclusion was that we really need to do regular code reviews. Three major issues we just fixed were of the "holy crap, who wrote that" and "why the hell is this doing that?" variety.. Something that would have been immediately spotted if someone bothered to look at the code (someone other than the author anyway). It's so easy to make dumbass mistakes when working under pressure and having a second set of eyeballs verifying your code can be more than just helpful.. it could save your sanity and embarrassment later.
One of the principles of extreme programming is to do something like this - pair programming. Having two people work on the same code at the same time. I've been subjected to that at work for about a week.. and it nearly ended in a homicide as I slowly learned to hate the guts of the poor co-worker stuck coding with me. I'm not really good at working in tandem with another programmer.. especially one that argues about my bracing style while sprinkling the code with tons of unnecessary empty lines and indentation that would make groucho marx go ballistic.. (and he wasn't even a coder.. )
So what's the solution? Code reviews.. weekly session, couple hours long (longer and we'll all fall asleep probably) reviewing randomly picked source from project du-jour..
I think this could really work -- we all get to spot our mistakes before they enter production and can lock ourselves up in blissful, headphone-induced isolation in the meantime. Perfect.
Comments
Pairwise programming would drive me crazy. I can't write code if there's someone hanging round in my office in the first place. It's a solitary puruit. Would anyone suggest "pairwise essay writing" as a sensible thing to do?
On the other hand, I do believe that every piece of code should be looked at by more than one pair of eyes. Not necessarily in formal code review -- that seems pretty unproductive -- but just as normal operating procedure. (I eyeball everything the other two guys on my sub-project write). Formal reviews should be reserved for the really hairy algorithms.
As far as brace placement is concerned, there's only one rule. If you originated the code, you get to pick the style. Thereafter, everyone who touches that module has to write code that's stylistically indistinguishable from the original.
Posted by: dave | April 9, 2003 08:54 PM
Code reviews can be an excellent tool.
We try to use them on our projects all the time but we rarely actually do due to time constraints.
The thing is, they have to be done *properly*.
And properly is *very* time consuming.
I can scan a class in a short space of time and probably only spot cosmetic issues (coding standards violations etc).
Spotting subtle logic bugs is another matter altogether.
Posted by: Darren | April 10, 2003 08:48 AM
The problem I find with code reviews is they tend to be the sort of thing everyone says they should do more of, but don't. When the project starts staring down the barrel of a deadline, code reviews tend to be the first thing to go. Everyone has better things to do (their own work) than go carefully over someone else's code. They're also incredibly annoying because they represent an unknown amount of re-work hovering always like a black cloud over your estimate.
I know this is probably a glib response, but one of the /other/ things XP tells you to do, is have coding standards strict enough that you shouldn't be able to look at a random piece of code and guess who wrote it. You always end up with a standard that nobody is entirely happy with, but it saves so many disagreements.
Some people, though, just don't work well in that situation. I'm in the middle. I would prefer to work alone, but I recognise I produce better code in a pair (so long as my pair is of similar ability to me), so I'm willing to put up with the annoyances because ultimately, I'm paid to write good code, not to have fun.
A possible compromise that I've seen work quite well is to ensure that programming tasks get shared around (so no one person is responsible for a single piece of the code), and develop a healthy environment for refactoring. If people are working with, and encouraged to look at the code that is already there, there is the natural programmer's tendency to "fix stuff that looks bad" as they go by.
It's not as rigorous as pair programming or a formal code review, but it's a good practical compromise that seems to work okay.
Posted by: Charles Miller | April 10, 2003 05:37 PM
Actually, I do my own code reviews; it works pretty well.
At the point at which the code is all checked in and supposedly done, I print out everything on paper (that's essential), take it home (that's essential), sit down away from all computers (that's essential), and just read the damn stuff. Laphroaig makes it a little more bearable.
---------------
This is probable the right place to quote Maurice Wilkes:
"As soon as we started programming, we found out to our surprise that it wasn't as easy to get programs right as we had thought. Debugging had to be discovered. I can remember the exact instant when I realized that a large part of my life from then on was going to be spent in finding mistakes in my own programs."
Posted by: dave | April 11, 2003 07:25 PM
Thoughts about pair programming. I wrote about this in my blog earlier on this year...
Don't get me wrong, working together as part of a team is absolutely essential and there are times when you have to pair up if only for debugging. However, in constructing brand new code - I'm not sure pair programming is always effective. The only beneficial situation I can think of is pairing up a junior developer with a senior developer acting as a coach.
The art of fine coding is like painting a picture. How many great artists can you name whom practised pair painting. "Oi, Monet give up the brush - you've been working on that stupid lilly for ages".. "Hang on Vincent, I've almost done - ah shit now, look you've made me do - I've only gone and painted outside the sodding lines!".
Posted by: Jason | April 11, 2003 09:49 PM