That’s OK, though! Code review is hard to do correctly, especially if you never start. We’re experienced code reviewers, and we still struggle with it — working with other people is never easy, and at its core code review is a social process for developers to help each other write better code.
Code review is an opportunity for us to spread valuable knowledge between our fellow developers. Let’s take it seriously.
There are all sorts of ways this can go wrong. Here are a few comments I’ve personally seen that I’d rather not have:
“We don’t have time for code review.”
This is the worst mistake you can make. There’s strong empirical evidence that code review catches more bugs faster than software testing at a fraction of the cost. Even worse, organizations that aren’t doing code review frequently aren’t testing either. It’s not the bugs that go away when you don’t catch them — it’s the users.
“Seriously? Do you even code?”
Don’t be a jerk. As long as your fellow developer is willing to work with you in good faith to reach a mutually agreeable solution, they deserve the benefit of the doubt. People are generally doing their best, in their own way, and insulting their capabilities just drives a wedge through your team that makes it impossible to reach the goal. Show them a better way, and accept that it will take time and repetition before they really get it.
“How dare you!”
Don’t take criticism personally. Not everyone is going to like or agree with your code. If you can’t take constructive criticism gracefully, you’re seriously hampering your ability to refine and hone your craft. You’ll never be great without honestly listening to what people have to say about your work.
“Only bad developers write singletons.”
Life is full of nuance. Not every problem is important, and every decision has trade-offs. Slapping together a singleton to magically share state between two parts of the system isn’t pretty, but rearchitecting the system to gracefully cover the corner case is an exhausting way to waste time and money.
“This is wrong.”
If it’s wrong, it’s wrong — but is that helpful? Code review is an opportunity to share valuable knowledge between developers. Why not help them understand the underlying principles, and then lead them to discover the problem on their own? At the very least, give the correct solution — but don’t expect them to get it right next time.
“This is ugly.”
Ask your users how pretty the code is. If the answer surprises you, you have some reflection to do.
“I don’t like this.”
This isn’t a wine tasting. Does the change meet the requirements of the ticket and all enforced standards of quality, maintainability, and style? If so, the code is better off with the change. Ship it!
“…OK, whatever…”
If you don’t understand the feedback you’re getting on your code, don’t just go along with it. Accept the possibility that you’re wrong, or that there’s a better way to do it, clearly articulate your original thought process, and ask clarifying questions. This is a great opportunity to learn from someone else’s experience.
“Just glance over it real quick.”
Everything is contextual, but on average the empirical evidence suggests review should take between 200 and 400 lines of code per hour for optimal results. Much less and you’re just going to end up fixing the issues in production at 10x the cost.
“I’m still not happy with this indentation.”
I’m still not happy my change isn’t merged! If you’re spending much (any) time discussing style, you’re wasting time — the trick to style is that it doesn’t much matter which style you choose, just that everyone uses the same one consistently. Set up an automated linter with the default settings for your language and save the discussion for actual implementation issues.
That’s it!
Thanks for reading, everyone! I hope this was helpful. Let me know in the comments if you’ve had similar experiences, or if I’ve missed anything you’ve personally struggled with.
Also, if you’re personally interested in topics surrounding code quality and software development, come see what we’re doing with CheckGit!
Oldest comments (0)