December 2009 Archives

Code Reviews Or Insanity: Pick One

| No TrackBacks
I used to go through the same experience once a week.  I'd open up some code that I wrote a couple of months ago, I'd read a bit of it to understand what I was trying to do, and then I'd conclude that I'm a complete moron.  
billydeesign.jpg
Based upon that crazy, kludgey mess I wrote just a few weeks back, I'd realize that, not only should I be barred from using a compiler from the rest of my life, but that I probably can't be trusted around shoelaces and sharp edges either.  Code like that is why they sell malt liquor.

Okay, most of my code wasn't that bad.  A great deal of it was fine, acceptable code.  However, when I looked back on existing code, I'd always find this one little corner of disturbing weirdness that just shouldn't be allowed to live.  At the time, I'm sure it made perfect sense, but looking later, I could only suspect that severe head trauma was involved.

Wouldn't it be great if we could catch ourselves each time we took a leap into Bizarro Code Land?  Wouldn't we save ourselves a great deal of frustration if these code abominations never made it into the codebase to begin with?  Wouldn't it be cool to look at old code and not have to stifle a sob?

The good news is that it's actually easy to spot these mistakes when you write them.  The bad news is that you're not going to be able to spot them yourself.  This, my friend, is why we all need code reviews.

Don't Be Afraid
Just the term "code review" gives some people the shakes.  
Pointy-Hair.jpg
It sounds like you have to present a detailed report (with footnotes! and a bibliography!) on every line in your application to a committee of pointy-haired bosses who are about to start flinging rotten fruit at you.  That's not it at all, though.

We're all into people over process, right?  A code review can be as simple as having someone watch over your shoulder for 45 minutes as you walk them through your work for the week.  No committees!  No lengthy reports!  No rotten fruit!  It's just you and someone else having a conversation about your code.

You may want something a little more formal if you're writing the firmware for the Space Shuttle, but simple and informal code reviews do wonders for the rest of us mortals. 

Good Code Reviews
What's the difference between a good code review and a bad code review?  In a bad code review, you leave feeling terrible about yourself.  In a good code review, you leave feeling terrble about yourself, but you also have a list of things to refactor.  Just kidding.

So what actually is a good code review?  For me, it all comes down to  a few practices that are easy and effective.

A good code review focuses on the code.  Some people seem to think you need to bring in all manner of class and sequence diagrams for a code review.  You don't, unless insanity by UML is your idea of a good time.  Just fire up the editor and start looking at code.

A good code review results in agreement.  There has to be give-and-take in a good review.  If you're reviewing my code, you can't just declare, "This sucks, rewrite the whole thing!"  Likewise, I can't just declare, "I hate you, you insufferable ass!  This code is perfect!"  A good review should produce a list of reasonable refactorings to make, and this should be a list that both the reviewer and the reviewee agree on.

A good code review is an intimate affair.  barry-white-1.jpgThat's not to say you should be doing code reviews in your underwear while listening to Barry White.  By intimate, I simply mean it should be you and one or two reviewers.  With a small group, you can go through the code quickly and easily, and the conversation doesn't spin out of control.  Otherwise, it's Review by Committee, and the Committee might quickly go from, "Hmm, this is interesting," to "Why doesn't our framework do this," to "Let's rewrite everything in Erlang!"

A good code review is limited in scope.  You've had too much Nyquil if you think you can cover an entire system in a 45 minute review.  Instead, focus on something that a person can reasonably understand in that time period, like one particular feature or a specific business process.  It's better to go deep than shallow.

A good code review isn't a one off event.  We want to do outstanding work and produce things that are so awesome that shiny medals and honorary keys to the city are in order, right?  You can't do that consistently if your code is reviewed once every 6 months.  Once you see how easy and interesting these simple code reviews are, weekly reviews with a teammate make perfect sense.

porky.jpg
A good code review has a good reviewer.  There's no point at all in doing a review if the reviewer doesn't know a string from Porky Pig's bowtie.  Pick someone you respect; it should be someone who's qualified to suggest changes in naming, variable scope, object oriented design, database schema, etc.  It should be someone who'll feel comfortable telling you, "What is this load of digital feces?"  Don't pick the intern.

The Final Practice
A good code review occurs.  If you aren't actively reviewing code, then stop reading this right now.  Walk over to the person next to you.  Ask them when they have an hour to look at some code.  If they are in the bathroom, sit there until they come back.  If they don't come back, knock on the bathroom door and ask if they need some Pepto Bismol.  Then ask if they have an hour to look at some code.

If you're scared by code reviews, it's only because you're not doing them.  Start today and help yourself build something great.

The idea of having fun and building something meaningful through software is much, much older than this little blog. Check out this quote from the dedication of Abelson and Sussman's absolute classic, Structure and Interpretation of Computer Programs:

I think that it's extraordinarily important that we in computer science keep fun in computing. When it started out, it was an awful lot of fun. Of course, the paying customers got shafted every now and then, and after a while we began to take their complaints seriously. We began to feel as if we really were responsible for the successful, error-free perfect use of these machines. I don't think we are. I think we're responsible for stretching them, setting them off in new directions, and keeping fun in the house. I hope the field of computer science never loses its sense of fun. Above all, I hope we don't become missionaries. Don't feel as if you're Bible salesmen. The world has too many of those already. What you know about computing other people will learn. Don't feel as if the key to successful computing is only in your hands. What's in your hands, I think and hope, is intelligence: the ability to see the machine as more than when you were first led up to it, that you can make it more. - Alan J. Perlis

Amen.

We all know and love the Joel Test, Joel Spolsky's quick quiz to determine the quality of a software team.  If you're searching for a job, it's a great idea to run potential employers through the Joel Test.

There's a problem with it, though.  Well, there are multiple problems.  First, who the hell cares about hallway usability testing?  Second, you're only checking for the good qualities of a team.  What about the horrendously bad qualities that will eventually drive you towards both insanity and a crippling dependency on cough syrup?  
10106989_0579f7be13.jpg
We need something like the Joel Test to measure the potential crappiness of a job, or else each of us might stumble into becoming Milton Waddams.

Ladies and gentlemen, I present to you the Codypo Test, aka 8 Questions To Identify A Lame Programming Job.  If you are in an interview, you give the company the Codypo test, and the job scores more than a 1 or 2, you need to pull the fire alarm and get the hell out of there.


1.  Would I be paid below market rates?

If they're looking for 10 years of hardcore, multithreaded C++ experience and they're offering 48k, these people have lost their minds.  Expect this sort of delusional thinking to appear in everything they do.

2.  Would I always be on call?

No one likes being on call, because as soon as you're on call, someone is going to page you at 3 AM on a Sunday because the Reset button on the support portal is a different shade of blue than they're expecting.  Occasional on call stints are both understandable and tolerable, but 24/7 is for doctors and exorcists.

3.  Am I the IT staff?

You are a programmer.  You make software.  You are happy to support your software.  
garfield_1.gif
This does not, however, mean you are a computational master of the universe, and just the guy to ask why the receptionist's laptop got all weird after she installed that Garfield screensaver.

4.  Would I work with a single monitor?

It's no longer 1998.  We don't have to stare at a single 17" boat anchor all day while rocking out to Smashmouth. You can buy huge, thin LCDs for $100 each.  If doubling your productivity isn't worth $200 to this company, then this company may just be a really elaborate practical joke played by an eccentric billionaire.

5.  Will I be maintaining any ancient system, and what's it written in?

If you dig deep, you may hear, "Yeah, we're going to get into Ruby on Rails, but first, we'll need you to bust out some VB 4."  It is never, ever that easy.  
rasputin.a.gif
That VB 4 system will refuse to die, just like Rasputin.

6.  Would my internet usage filtered or monitored?

Programmers solve problems, and to solve problems effectively, you need resources.  The internet is the single greatest usage available.  Any company that refuses to accept that and blocks you from Usenet/ Google/Stack Overflow/whatever is treating like you a child/deranged porn fiend.

7.  Would I be the only programmer?

More than anything else, I have grown as a programmer thanks to my peers.  We answer each others' questions, we review each others' code, we whiteboard together, and we come up with creative insults when somebody breaks the build.  If it's just you, you're not going to get any technical feedback and you're not going to get any better.  Also, you'll only be able to insult yourself when you break the build, which might get you reported to HR.

8.  Am I expected to travel every week?

A bit of travel is necessary from time to time, particularly for face to face meetings with customers or offsite colleagues.  However, expecting you to leave your home every farging week is absurd.  We have the internet now; it allows us to communicate virtually.  Let's use that instead of me becoming BFF with the night receptionist at the Best Western.


I think those 8 questions constitute a good sanity test for a potential job.  Sure, there are valid reasons for a job to score a 1 (eg, you might be the IT Staff or on call all the time at a startup).  However, once you get past one Yes to these questions, it's time to leave the interview through any means necessary, including faking a heart attack.

No matter how great the potential projects and teammates might be, I don't think you can do truly meaningful work in an environment where you, the developer, aren't empowered to succeed.  If a company doesn't get that, then they don't get software.
What in the heck is delightful software?  If you'd asked me a week ago, I would've said it's either the code used by Santa's elves to produce candy canes or the OS on Spongebob Squarepants' cell phone.  At first glance, the term is slightly ridiculous.
delightedspongebob.jpg

Think about this, though: software can be fun to make and use.  The code can be well-designed, covered thoroughly by unit tests, and utterly devoid of broken windows; the codebase can actually be a treat to contribute to.  The team working on the code can be upbeat and helpful.  When the users spend time with the product, they might feel just a little bit of life's stress melt away, knowing that this product will do something useful for the user without making the user think.  The software, in short, is a delight.

The delight doesn't come from the project itself.  After all, who giggles with glee when they're thinking about a billing system?  No, I think the pleasure here comes from attitudes and practices that we and our teams can adopt towards our craft.  Once adopted, the mere act of creating and using software becomes both more fun and more meaningful.

That's what we're going to talk about here.  We're going to talk about a scalable, fruitful, and rewarding way to build both software and a life in the software industry.  Once we cover that, we will then and only then investigate who wrote the OS on Spongebob Squarepants' cell phone.  (Preliminary guess: Nokia?)

About the Author

The Art of Delightful Software is written by Cody Powell. I'm a dev manager at Amazon where I work on the Instant Video, Mobile Clients team. Before that, I was CTO at Famigo, a venture-funded startup that helps families find and manage mobile content.

Are you interested in building great mobile apps for Amazon Instant Video? Email me!

Twitter: @codypo
LinkedIn: codypo's profile
Personal blog: Goulash
Email: firstname + firstname lastname dot com

Categories