Code Review Tips for Junior Developers
Why review code?
I perform code reviews because:
- It gives me practice reading code.
- I learn more about the code base.
- I learn different styles and approaches to writing code.
- I understand what people are working on and what is changing.
- It helps my teammates improve their code, catch bugs, and get their code through the door.
- It earns respect from teammates.
Hopefully, you’re convinced you should make code review a habit. However, the biggest hurdle probably isn’t motivation, but uncertainty on how to get started.
Tips for reviewing code
Reviewing code when you’re starting at a company can feel daunting. Not only because of “imposter syndrome”, but also because you might not know enough about the code you’re reviewing to recognize anything that could or should be changed.
Here are a few things you can do when reviewing even the most unfamiliar code:
- Ask what the code is doing. Take the time to understand the code and read it line-by-line. If you come across something you don’t understand, look it up or sift through README’s and any other documentation. There may be no such thing as a “bad question”, but there are lazy ones. If you still don’t understand something after doing your due diligence, ask the code author if she can explain the line(s).
- Ask why the code is written the way it is. You might understand what is happening, but why is it implemented that way? Why is the variable/function named that way? Why is that a list rather than a set? Does it make sense? Is there a better way? Writing code requires a lot of decisions and it’s easy for the author to overlook small things while he’s focusing on getting the code to work. An extra set of eyes asking “why?” can help the author identify things they’ve missed.
- Ask that a pull request be kept small. If you’re glancing through the files comprising the pull request and thinking to yourself, “Do I have to look through all this?” then there’s a good chance that the pull request is “too big”. You aren’t being lazy: large pull requests are bad not only because they’re a pain to review, but also because they are inherently riskier. While there may be a good explanation as to why the pull request is so big, it is best for everyone if you at least ask if a pull request can be broken down.
- Ask for a good commit message. Not just for your sake, but future generations (including the author himself!). It’s easy to forget why even your own code was changed, let alone someone else reading your code and trying to understand it. A good commit message not only outlines what was done but why the change was made.
- Ask for tests. Most organizations would agree that it is best to include some form of tests with (almost) every change. If you don’t see any tests in the pull request, ask if there should be (I say “ask” because there may be a good reason not to include tests). If tests are already included, you can also help out the author by making sure that the critical code paths and edge cases are tested.
- Look for bugs at the “micro” level. Should there be a null check? Does the if-statement behave correctly for all permutations? Is there a return statement in the wrong place? Make sure the line-by-line logic makes sense. This doesn’t require a knowledge of the overarching codebase: any developer should be able to catch these types of problems. If you can spot a “micro” level problem, it probably means there’s a missing test case that could be added.
- Look for bugs at the “macro” level. Reason whether or not the change to application logic may have unexpected consequences. These types of problems require high-level reasoning and are often harder to spot. Examples might include: Should we be removing the message once it’s been read? Does changing this name break a pointer in some other part of the application?
- Read “Clean Code”. It outlines best practices for writing maintainable code. Most of the comments in my code reviews derive from ideas in this book, and I highly recommend it to anyone who works in code. If you or your teammates disagree with the paradigms it suggests, find your own philosophy. Either way, I think it’s important for you and your team to be opinionated and aligned on what makes for clean, maintainable code, and you can help enforce it through reviews.
- Practice good etiquette. Reviewing someone’s work is a personal process, and criticism (constructive or not) can be difficult to digest. Help the author out by softening your request/comment, whether it’s a fact or opinion. A few strategies for doing this:
1. Frame suggestions as questions. “Maybe it would be better if…”, “What if this was done this way…”, “Do you think it might be better if…”.
2. Keep it impersonal. Don’t say, “I think you did this wrong” or, “You could’ve done this better by…”. Instead, say, “I’m not sure this is correct” or, “I think this could be better”. Generally, leave out the word “you”.
3. Only block a pull request if you think there will be a bug. Don’t block pull requests because of an opinion.
4. Be polite. We’re all busy adults, but “please” and “thank you” still go a long way. Whether you’re reviewing code, or asking for a review, being appreciative for someone else’s time and effort is always a good idea.
I encourage every developer to review code, especially junior developers. Performing code review as a junior developer is a great way to learn and earn respect from senior developers.
Asking questions is my go-to when reviewing unfamiliar code. Not only does it help me learn, but it forces the code owner to think critically about her code, which may lead to new insights or bug fixes.