This site is my tutorials, rantings and musings on software development, with a focus on JavaScript, Ember.js, Vue.js and Node.js.

20 Apr 2015

When Should You Do A Code Review?

Recently I read a post about code shaming by David Robinson. He makes the point that everyone has written bad code in the past and therefore we should be more thoughtful in the criticism we give others when reading their code. The post was inspired by this XKCD comic. He concludes the post by asking the reader to consider how they would have reacted if the code you wrote was mocked while you were still learning.

John Carmack, now with Facebook, weighed in on the conversation in the comments and recommended that people should separate themselves from their work. He suggested that everyone should be open to criticism of their code because it makes us better coders.

I agree that to grow and learn, as human beings, we need to be able to take criticism. At least constructive criticism. This is especially important when you are learning to code. On the other hand I don't think I can easily separate myself from the code I write so I can sympathize. I take pride or shame in it, for better or for worse. I've had people mock my code and it takes discipline to listen to their suggestions and make changes.

Code Reviews

This got met thinking about code reviews. At my work I deal with a large Java code base. We take code quality serious at my work and we do peer reviews after every check in (at least one other person has to look it over). Occasionally we do informal team code reviews where we all sit down and we discuss the code base and major features. I'm not sure if this would work for all places, however it works for us.

Stack Overflow has a post on this. The top voted answer suggested that code reviews should only occur after code is pushed to the mainline branch which should have the highest level of quality code. The automation tests (you do have automation tests right?) should be ran before the code review and then again after it's pushed to the server. There is lots of tools out there that can help with code reviews, that make this process easier. The difficulty of doing code reviews should not be an excuse not to do it.

What About Code Shaming?

Having a work environment where everyone trusts each other and is open for criticism is probably rare. We can only worry about ourselves and take responsibility in the code we write. I know I hate being the person that broke the build server and trying to fix it afterwards is not fun. Taking initiative, keeping an open mind and working with more senior developers is a good start. Conducting thorough code reviews will only make us better programmers. Even if it might hurt our feelings, we should all strive to do better.

I haven't participated in any open source projects yet, although I imagine that this isn't much of a problem in that community. Everyone understand that they are contributing to the greater good and if your PR gets rejected after being reviewed then that's OK. You fix the problem and resubmit it. We should take the same philosophy with all code we work on.

Future

I think the final answer is it depends. However we should not worry about others criticism as a reason not to do code reviews or share our code at all. Perhaps in a future article we'll deep dive into this topic more.