On The Importance of Code Reviews

From my "I wrote this a quite a while ago and never got around to posting it" collection...

I recently reviewed some code that contained a number of shared, global variables. Basically, a shared variable is re-used by all callers to the particular program in which it is defined. If one caller sets the value to “foo”, every other caller to that program will see the value “foo”. You could think of it as a chunk of shared memory that can be accessed by any instance of the program in which the variable is defined. A real world analogy might be an office bulletin board where every worker has access to read and post important news. That works fine if only one or two people are involved in changing the items posted to the bulletin board at any one time, but what if 100, 1000, or 10,000 people all tried to update the posted news items at the same time?

The shared variable feature of VB.NET can be used to great advantage because it allows us to perform long-running tasks only once and share the results across every caller. Thus, all but the very first execution will run much faster than they would if the long-running task had to be performed every time. That said, we also need to be very careful when implementing this technique (search for SyncLock for more information).

In the code I reviewed, every execution of the program was changing the values of the various shared, global variables, and this introduces a major issue. The problem is that every caller has the potential of overwriting the data of every other caller at points in time when this will cause errors or erroneous information to be returned. Due to the way applications are typically tested prior to reaching a production environment (very few users performing any given task), it is unlikely that such an issue would be found prior to it impacting end users. That is, unless you hold a code review attended by people that can find such a flaw and help come up with a solution. If the code in question had not been reviewed, I can say without a trace of exaggeration that the result would have been weeks to months of customer complaints about strange, intermittent errors. Numerous hours and dollars could have been wasted trying to track down and fix the problem. Instead, the problem was resolved prior to ever making it outside the development environment. This is just one reason why code reviews, when done properly and involving the appropriate participants, are an invaluable part of the development process. Every project should allow time in the schedule to both hold and react to the comments from code reviews.

It is important to note that a code review does not always have to be what one might typically envision (multiple people locked in a room for 60-90 minutes). The idea is just that every piece of code we write has been seen by at least one more set of knowledgeable eyes prior to being thrust upon the users. The number of reviewers should scale up for code that is more critical, complicated, or involves the security of a user's account and/or private information.