Thursday, July 5, 2007

Vacuum Coding Syndrome: The Need for Peer Review

I stumbled across this interesting quote today as I was looking for information about the effectiveness of peer code reviews:

There are some thing's you can't unit test. You can't unit test design or architecture. And sometimes you can get unit tests wrong—you might assert the wrong things. There is always a place for human eye-balling of code.

Matt Quail, at JavaOne 2007

This statement resonates profoundly with me because of the situation that I'm in. As I've mentioned previously, I code alone. I have no peers with whom I work and can share designs and submit them for review. So when I make a bad design decision and then implement it, I'm stuck with it. Lately, I've been going over my code, and when I come across some of my older stuff, I'm frequently left wondering just what in the hell I was thinking.

There are designs and architecture decisions that I made three years ago that seemed perfectly reasonable at the time, and now I look at them with abject horror.

In one case, I knew that our database architecture was going to be massive, and that it was going to be changing frequently. I also knew that the data model classes were going to have to be constantly updated to stay in synch with the database, and that there was no way I was going to be able to do all that manual work without missing something. I needed to be able to update the database schema, push a button, have the classes regenerated, and then move along to focus on the business logic.

So I wrote a program that did that. It was, to be sure, a very fast, very efficient program. But it didn't generate very pretty code. In fact, it generated a lot of classes that we never use. For every table, it generated an insert, update, delete, exists, delete all, and select all stored procedure. For each stored procedure it had generated, it created a wrapper class that correctly created the SqlCommand, populated its parameters, and invoked the procedure. It also generated a data model class and a collection class. It also generated a strongly typed primary key class, and the DAC class that invoked the stored procedure classes.

Now that's a lot of code. At the time, it seemed like an act of sheer brilliance, and I marveled at the fact that if the schema changed I could simply regenerate the classes and know that they matched the schema. But the number of stored procedures quickly grew out of proportion to what we actually needed for the system we were building, and the vast majority of them were never used. And if those procedures were never being used, you can bet that the generated classes were never being used, and neither were the methods on the DAC classes.

That program, thankfully, was decommissioned early on, but it left a massive amount of code in its wake. Newer code no longer follows the model that it established, but is, in fact, leaner, smaller, and uses far fewer classes and stored procedures. It's a joy to work with the new code. But I groan and cringe every time I have to wade through that older code. 

It was a case of very efficiently created code bloat. It would very likely have been stopped early on had someone been there to either (a) review the design of the code generator, (b) point me to an existing tool that was better at it, or (c) convince me that we actually had time to just do it the right way in the first place.

In any case, I'd never do it that way again. I have been duly chastised by my own foolishness.

I'm absolutely certain that I'm not the only one suffering from Vacuum Coding Syndrome. I'm equally certain that I need to have someone reviewing my designs and my code. So as I sat chain-smoking today, looking at a piece of paper with an abominably badly written piece of code printed on it that I knew I had to fix, I realized that I really needed to start submitting my designs for peer review to someone. Anyone, at this point, really. Because I really need to stop making silly mistakes. I'm the one that has to live with them.

So I got to thinking about it. In the past, I've submitted small snippets of code onto Usenet for review, and the response has been mixed. I've submitted some of my good code onto The Code Project and the response has actually been pretty good (but that code works). Now my brain is churning over a different kind of forum.

What I need is a forum where developers can submit code and/or designs for peer review. That would be the whole point of the forum. The online community is overflowing with professionals who are far more experienced, far more intelligent, and far more capable than I am. It would be a great boon to lone developers if there was a site designed specifically to allow us to post our code samples (sans IP) and designs (in some ubiquitous format) for peer review. 

I've seen products like CodeCollaborator, which foster collaborative code reviews and allow users to work together on code reviews using software. You can chat, compare notes, diff the files, and so on. Why hasn't anyone thought to do this on the Web? I'm not envisioning everything that CodeCollaborator does on the Web; what I am envisioning is a simple forum, like The Code Project or Construx's Software Best Practices.

So here I am, scouring the web, looking for such a resource. If any of you happen to know of one, I'd be much obliged if you could point it out to me. I'd roll my own, but given my history of making poor design decisions, and my lack of access to resources capable of reviewing my designs...

Well, you know. <shrug/>

3 comments:

Anonymous said...

I like the thought of it, but I wonder of the utility. Of course, it could be a great resource for the lone hobbyist, but how many people work with proprietary code that they can't post online?

I guess keeping it high level might work there, but in some places even that may be too much.

Mike Hofer said...

I don't think it's all that difficult to mock up many samples to protect intellectual property.
My primary concern, for instance, is in my designs, as opposed to my code. I think my code is pretty solid (although those may be famous last words).

For example, I use a specific coding pattern when accessing stored procedures in the database. I think it's a good design, but I'd really like to have other developers review that design for potential problems. It would be a snap to mock up a sample of that that didn't involve any proprietary data, and have my online peers take a look at it.

The problem, as I see it now, is that the only really effective place to do that at this time is either here (with a very low readership), or on Usenet (where things quickly devolve into a flame war over frequently irrelevant issues).

A forum dedicated to peer reviews, on the other hand, might facilitate that sort of thing quite well. If you were asking someone to review your pattern, that might actually be something of great value.

Anonymous said...

>The online community is overflowing with professionals who are far more experienced, far more intelligent, and far more capable than I am... for peer review.

You only need other people with different experiences to do a good review. Peer review is from your peers, expert review isn't required.

The discussion that goes with code review can teach you stuff the experts never thought of.