Prerequisites for Effective Code Review

Editorial Note: this is a post that I wrote originally for the SmartBear blog.  You can read the original here, at their site.  You should check out the posts by some of the other authors over there as well.

I’m betting that, at some point in your travels, you’ve seen a blog post or a document in a software shop that details prerequisites for good code review. It probably contains items like, “make sure you’ve run all the unit tests” and “make sure you’ve given the reviewer(s) enough notice.” This information certainly has value, but it’s not what I want to talk about today.

Today, I want to talk about prerequisites for effective code review culture, rather than prerequisites for an actual code review. The question thus is not, “are you prepared for this code review?” but rather “is your group prepared for code review, in general, to be as effective as it can be?”

Asking a question like this makes sense, if you stop and think about it.  A director of software engineering can’t one day stroll through her group’s team space, declare, “Alright, folks, start doing code review,” and expect awesome productivity to follow straightaway.  Various elements of preparation are necessary, first.

Read More

What To Avoid When Doing Code Reviews

Editorial Note: I originally wrote this post for the SmartBear blog.  You can see the original here, at their site.  Go on over and check out their site!

Years ago, I had a senior software developer role in a shop where code review was part of the standard workflow. The way the review process worked was that anyone writing code had to submit the code for review to one of the senior developers of their choosing.

Over the course of time, I began to notice something interesting and a bit flattering: I was picked a lot to do reviews.  When I first got an inkling of this trend, I simply thought I was flattering myself, but then I started to keep track and I realized that there was a definite trend.  What was going on?

Was I an “easy grader” or a pushover?  Was it just by chance?  Was it that people thought I was some kind of legendary, super-developer?  Turns out it was none of these things.

In search of the answer, I started to pay more attention to the nature of code reviews offered by other senior developers.  In doing this, I came to realize that the answer lay not as much in what I was doing, but in what they were doing.  Specifically, they were doing things that I wasn’t doing, and those actions were causing others to seek out my reviews.

This phenomenon was revealing to me, so I made a point to pay attention to the actions of different reviewers among the senior developers, and line them up with how much people sought out or avoided them for code review.  Making these observations taught me valuable lessons about what to avoid when doing code reviews.

Read More

Avoiding The Politics of Code Review

Editorial Note: I originally wrote this post for the SmartBear blog.  You can check out the original here, at their site.  They have a number of authors over there that are writing good posts.

Asking someone to look over your work is a pretty common practice in any discipline. We’ve been conditioned with the idea that it’s always good to have a second set of eyes look at your work prior to officially marking it as done.

Writing and reviewing code is no exception.  It’s pretty easy to forget to check an argument for null, so having someone review your work only makes sense.  In fact, it makes so much sense that software departments tend to formalize and mandate the process, calling it “code review.”  And the practice is quite effective.

But formalizing and mandating a process in a corporate setting can lead to a variety of unintended consequences.

Read More

Your Coworker’s Bad Code: Having The Hard Conversation

Editorial Note: This post was originally written for the SmartBear blog.  You can check out the original here, on their site.  If you’ve never had the chance, take a look at their blog in general.  A lot of good authors over there.

Last time, I talked about how to prepare for a tough conversation with a coworker about having bad code.  This included understanding what not to say and creating a game plan of specific shortcomings to address and concrete outcomes you want from the conversation.  This time, I’m going to talk about how to actually engage with your teammate, who I’m calling “Bob.”

Engage

Having built your case ahead of time, it’s time to go have a chat with Bob. You’re calm, you’re rational, you have a legitimate argument, and you’re all set for a constructive dialog…but the lead in for the conversation threatens to be awkward. What I’d suggest doing to put the conversation in more natural terms is to ask for his help. “Hey Bob, I’m chasing a defect through the code and it led me to this method of yours. It’s a little hard to follow at first glance, so I was hoping maybe we could trace through it together?” Now you’re not coming over to preach to Bob about the evils of his code but rather to ask him to help you solve a problem.

Once you’re looking together at a screen and starting to dig in, one of the most effective ways I’ve found to surface code problems is through the Socratic Method. Instead of telling Bob that the method is too long, ask a series of questions. “Wow, good thing you’re here—this is a pretty long method with a lot going on. How long do you think it would take the average team member to understand it?” “Huh, wow, three or four hours seems like a pretty long time to spend trying to understand a method, don’t you think?” “What if it were smaller?”

Making proclamations of fact or strongly stating opinions tends to put people in a defensive posture. Asking questions, even leading ones, doesn’t get people’s hackles up as much. They tend to join you in problem-solving mode rather than argue against you in debate mode. Still, asking questions this way may not lead to the desired outcome, which is why you’ve done your homework. Switch gears from the questions to statements of your experience and how you feel. These are inarguable. Read More

How to Address Your Coworker’s Bad Code (Part 1)

Editorial Note: This post was originally written, some months back, for the SmartBear blog.  You can read the original at their site, and. while you’re at it, I recommend checking out other posts there as well.  There are a lot of good authors over there, and I’m flattered to be included with them.  This is part 1 of 2.  I’ll publish 2 here later as well, but it’s already published on their site if you want to read it immediately.

Ugh. You’re sitting at your desk, trying to chase down a bug that’s been reported, when it happens. The hunt takes you into some method that inspires you to do a double take. It’s about 1,200 lines long, it has switch statements nested three deep, and you think (but aren’t sure) that it does the same thing two or three times in a row for no particular reason. You look at the source control history, see that this is another “Bob special,” and start thinking about finally having a long overdue talk with Bob so that you don’t have to keep cleaning up these messes. That sure won’t be a fun talk. So how do you approach it?

Read More