Philipp Hauer's Blog

Engineering Management, Java Ecosystem, Kotlin, Sociology of Software Development

How You Should Consider Code Reviews

Posted on Feb 8, 2015. Updated on Jan 20, 2019

From time to time I review code of my colleagues. Especially for a younger developer giving feedback to an older and experienced developer is hard. The colleague has to be open-minded for feedback and criticism. For this, it’s helpful to communicate how code reviews should be considered. I’m convinced that doing so will increase the acceptance of your feedback. But let’s get more concrete.

Update 2018

3 years after this post, I wrote a new post Code Review Guidelines containing the content of this post in an updated and modernized way. Additionally, the post covers advices and phrasing techniques for the reviewer. I strongly recommend to read the new post instead of this one.

Everybody’s Code Can Be Improved

Code reviews are not about assessing the programming skills of a colleague, it’s about improvements. You should consider code reviews as an opportunity to improve your code quality. Therefore, you should be thankful. And – most important – everybody’s code can be improved. Even the code of a “senior” coding guru who knows every trick in the book can be reviewed and improved. Let’s see why…

New Perspectives on Your Code

While writing code you see things through your eyes and with the background knowledge only you have. But the other guy who has to read, understand and maintain your code doesn’t have this background knowledge. He sees the code through this own eyes – the eyes of the maintainer. Things which make perfect sense for you as the initial developer can be extremely cryptic for the maintenance guy.

A code review provides new perspectives on your code

A code review provides new perspectives on your code

It’s about implicit knowledge. Take a look at this simple example:

if (table.getValue() == null || table.getValue() instanceof TemporaryRowId) {
  return Optional.absent();
}

You know that getValue() returns null, if the user has nothing selected in the user interface. Moreover, it’s clear for you, that if getValue() returns a TemporaryRowId the selected item has not been persisted. But does our maintenance guy have our implicit knowledge? In a code review session with him you get the opportunity to become aware of the lack of explicitness. But that is nothing to worry about. You get the chance to look at your code through the eyes of a person without your background. That’s just great! Code reviews provide new perspectives on our code and reveal implicit knowledge.

Let’s transform implicit to explicit knowledge by refactoring the code:

boolean isSomethingSelected = table.getValue() != null;
boolean isNotPersisted = table.getValue() instanceof TemporaryRowId;
if (!isSomethingSelected || isNotPersisted) {
  return Optional.absent();
}

The above example is a technical one and refers to framework knowledge. But the same is valid for domain knowledge.

You Are Not Your Code

“Criticism is almost never personal in a professional software engineering environment — it’s usually just part of the process of making a better product”. Fitzpatrick, Collins-Sussman: Debugging Teams

Code reviews revolve around code, not people. Two aspects have to be taken into account:

  • The way of giving feedback is extremely important (I will write about this in a later blog post).
  • Programming is just a skill. It improves with training – and this never stops. If somebody points out how you could improve this skill, would you take it as an attack on you as a human being? No. **Don’t connect your self-worth with the code you write. You are not your code. **You are still a valuable team member and human being even if there are mistakes and bad practices in your code.
You are not your code. Keep this in mind while getting feedback in a code review.

You are not your code. Keep this in mind while getting feedback in a code review.

If you are interested in this topic, I recommend you to read the great book Debugging Teams from Fitzpatrick and Collins-Sussman.

Source of Best Practices and Experiences

This is maybe an obvious point, but very important. The guy giving feedback simply knows things you don’t know (for instance best practices, coding techniques, design patterns, useful library methods, domain constraints, better approaches to solve a problem). Maybe he works longer in the current project or is an expert in this field. Consider him as a valuable source for improving your code, your knowledge and programming skill.

Humility

Be humble! Nobody’s perfect and everybody is making mistakes. That’s ok!

Making mistakes is human.

Everybody can improve. Don’t consider yourself as infallible. Don’t infer your professionalism and reliability as a software developer from infallibility and flawlessness. Even if you are a respected and experienced software engineer, it’s totally ok to make mistakes or asks others for help. This doesn’t decrease your value as a team member. In fact this shows that you are really professional, honest and after all a human being.