TURBU Tech

I got a request at work yesterday to look at an issue one of our clients was having.  A certain service didn’t seem to be doing anything at all, with no failure messages in the server logs.  After asking a few questions and checking out the log, I determined that the initialization code for that service had never run when the server started up, so I pulled up a local copy on my dev system to figure out why.

I put some breakpoints in the initialization code and started tracing through to figure out what was going wrong.  Partway through, the initialization tripped over a certain (unrelated) section of code.  After a bit of checking, I determined that the problem was related to the way my dev machine was set up, and not to the code itself; this wasn’t the bug I was looking for.  So I commented it out and restarted.

It didn’t take too much more work to find the source of the problem.  At the very start of the initialization routine for the service in question, we had some code that was handling some date/time parsing in an incorrect way, which caused it to raise an exception that was never being caught.  Once I knew what the problem was, it wasn’t difficult to fix, and so I coded up a fix, tested it, and went to check it in once I was sure it was working right.

As part of my normal routine for checking things in, I went to review my changes.  They were pretty small and simple, but it’s basic discipline that I learned years ago: Always Review Your Code Before Checkin.  I asked a coworker to look at it with me.  On personal projects, I just review by myself, but I always make sure to do one.

And I’m sure glad I did.  When I pulled up the diff in BeyondCompare, there along the side were marks for changes that I expected, plus a big red section that I wasn’t expecting at all!  I clicked down to it and was a bit surprised to see the section of code I had commented out.  Actually finding the real problem hadn’t been difficult, but it had taken a while due to having to wait for the server to load a bunch of data from a remote database when I restarted it, and it had been long enough that I’d forgotten about that little detail.  It could have caused some ugly problems for the client if I’d checked that commented-out code in and it made it into their next build!

This isn’t the first time I’ve caught a mistake like this, either.  I don’t do it often, but I do it often enough to be aware of the value of code review.  For me, it’s a pretty simple matter: before you check anything in to source control, open every file in a visual tool that will point out everything you’re changing.  I prefer BeyondCompare, both at work and for personal projects, because it’s quite simply the best tool for that particular purpose that I’ve seen, but if you can find something that works better for you, use that.  (And if you can’t, use BeyondCompare.  It’s not free, but it’s not expensive by any means–even for the Pro Edition–and it’s worth every penny and then some.)

When you’re looking at your code and you have your changes highlighted, it gives a certain sense of clarity that can sometimes get lost looking at the whole file (or group of files) in the IDE.  You look over your changes, make sure they make sense, and make sure you’re not mixing partial changes from multiple features or fixes into one commit.  (Sometimes you’ll be halfway through something, then have to fix something else that’s mostly unrelated but touches on one or more of the same files.  TortoiseSVN recently added a feature that makes this scenario much easier to deal with: “Restore after commit”.  You mark a file as “Restore after commit,” use BeyondCompare to revert the unrelated changes, and then commit, and once the checkin is finished, it puts the half-finished changes you reverted back, so you can continue to work on your code.)

Reviewing can also be useful during the development process.  Occasionally one of my coworkers will ask me over to his desk. “Hey Mason, can you look at this with me? I need another pair of eyes.”  And likewise, sometimes I’ll ask for help from one of them.  It’s usually for tricky code that will be difficult to get right, particularly if there are time constraints involved.  (Management, clients, or both, breathing down your neck about fixing a critical bug, for example.)  And it can be very helpful.  We can catch each other’s mistakes, or provide insights that the other might not have thought of, just because of having different backgrounds and skillsets.

I’m not going to advocate full-on Pair Programming as a general development practice here, but there are definitely times when it comes in handy!  If you do this, though, it has to be with the understanding that you’re working on something as equals, and you’re both free to disagree and criticize (politely) and have your ideas listened to.  Occasionally I overhear coworkers working on something together arguing back and forth over whether some thing will work right.  When it gets bad, sometimes I’ll snap at them and say “just run the code and see what it does!”

As always, comments are welcome.  How do you handle reviews and joint development where you work?  How do you handle it for personal projects?  Have you found any specific techniques that come in handy?