Once you have a strong foundation for being a realistic code reviewer, you’re finally ready to move into the actual code itself.

Rely on established patterns more than personal style

A common mistake in a code review is recommending things you’re used to seeing rather than well-documented patterns. The problem with this approach—besides reflecting a lack of thoughtfulness or desire to find the best solutions—is it can create a lack of trust. If your ego and personal preferences get in the way, you’ll lose the trust and confidence of the author—and the code suffers as a result.

Never forget: the perspective we offer should be a helpful flag for the author, not simply an opinionated comment from someone who isn’t the one actually writing the code.

A good example we encountered at Eventbrite is the JavaScript Switch Statement. Consider this piece of code:

// given a state, we return our city of choice. 
function getPreferedCitiesByState(state) {
  switch (state) {
    case "Florida": 
      return "Tallahassee";
    case "Idaho": 
      return "Boise";
    case "Arizona": 
      return "Phoenix";
    case "South Carolina": 
      return "Columbia";
    default: 
      return "San Francisco";
  }
}
// we could separate logic from information. using a dictonary in C# or map in python. in js would be a simple Object. 
function getPreferedCitiesByState(state) {
  var preferedCities = {
    "Florida": "Tallahassee",
    "Idaho": "Boise",
    "Arizona": "Phoenix",
    "South Carolina": "Columbia"
  }, 
  defaultCity = 'San Francisco';
  // we lookup in the dictionary/object/strcuture for the matching key, if non is matched, we return the defaultCity
  return preferedCities[state] || defaultCity;
}

When I look at this code, I immediately think it can be more succinct, simplified, and written in a way that’s much easier to maintain and improve in the future. But how should I communicate that with an author?

“I don’t like that statement; use this piece of code instead.”

Sure, that might get us to a better piece of code for now, but at best it improves the code without improving the author and at worst, it becomes a frustrating debate of egos.

There are plenty of good alternatives for how to approach giving productive feedback:

• Present the pattern (this feedback technique is heavily used and referred to as HashMaps or lookup maps in languages like Python or Java)
• Apply a well known principle called “tell, don’t ask” principle, and with it all the proper justifications since it’s well documented.
• Provide a generic solution that can be a model for resolving the specific trouble. This is especially useful if the solution isn’t common or specific enough to point out directly to it. Plus, the generic model can be a useful tool in the future as well.
• Ask the author questions that focus on the problem trying to be solved (like
  “How are we thinking about maintenance/scalability?”) rather than the specific current solution. This can help expose actionable points.

Move away from the framework to gain perspective

First, let’s clear the air: this doesn’t mean we recommend always write code outside of the chosen framework/tooling as possible. On the contrary: we should know what is tightly coupled with the framework and what’s not, and this creates separation in our code.

This will improve our maintainability when the time comes to upgrade the framework or even to switch framework without rethinking algorithms.

When reviewing code, finding this clear line (and asking the author for a clear division, if possible) makes maintainability possible or at least, if you encounter issues, narrowing down the actual problem easier.

Another way of presenting this concept would be asking for a clear API in every minimal abstraction. To assess if this is being achieved, stop in any part of the code, and ask:

• What is being transformed/altered here? (Ideally, it should only be one thing.)
• What is used to transform/alter the element? (Ideally, this would be a clear API.)
• How many flows can I see being developed here? (Ideally, it should be one.)

Even though this technique produces really clean code, the risk is overdoing it and creating over-engineered code—arguably, rendering our entire effort useless.

Standardize repetitive pieces of code

You might have heard quite a bit about “functional” or even “declarative” programming. (If not, DRY is a good starting point for learning the key concepts of these approaches.)

If we’re going through a code review and see several implementations that look alike but with tiny differences here and there, it can be really hard to keep track of them and how they affect the code.

At this point, we should start asking for utilities to simplify the code—or at least a way of finding the common denominator abstracted out. Once we isolate the abstraction, we can name it, which allows easy discussion/revisiting of the concept later on.

“Ship it!” with confidence

Here at Eventbrite, a great review always ends with a celebratory “ship it!” from whoever reviewed the code. The collective review team’s consent and blessing should be part of our great product.

Remember, code review will always involve compromise—it’s a matter of making the right compromises. You want to get everyone to yes, but it might not always be an excited yes.

The crucial thing here is that disagreements can be voiced, explored, and resolved. If you find yourself uncomfortable shipping a piece of code as it stands, it’s your responsibility to dig into why (and the earlier in the review you deal with this concern, the better).

First, check your ego to ensure the discomfort is valid, not just you digging in your heals. If you objectively feel there’s still a problem, now your job is to explore it with the author and/or other reviewers. You can:

• Go back to the core questions that informed the problem/solution earlier on.
• Ask for testing on the area you’re concerned about.
• Encourage the author to ask you additional questions about your feedback.

This is a conversation with someone whose instinct might be to protect what they’ve created, so don’t forget the importance of empathy. In the end, you should both have the goal of producing the best possible code—but it might be more difficult for them to critique it harshly.

As we grow as engineers, we discover new and fun ways to do this important work to create the best code and experiences on the web. This is how we make sure we excellent code as best as we can, and make a difference where we come out better every time. What’s yours?