10 thoughts on “Effective Code Reviews”

  3. That’s a well written one. Peer code review is like double-edged sword. your article clearly explains how to make use of this sword in proper way.

    Reply

  4. Few more bits that make code reviews less painful:
    * Review early, review often. Smaller patches are easier to digest.
    * Avoid review fatigue. Anything longer than an hour, and reviewers become buggy. Take a break. Or break a review into sessions if necessary.
    * Another way to avoid review fatigue: have a rotating list of reviewers. Don’t make the same person review consecutive patches if possible. (Bring someone over from another team if necessary.)
    * In-person / group reviews can foster good discussions, but offline, independent reviews allow reviewers to work on their own schedule. Not forcing a “context switch” onto a reviewer can make her more productive overall.
    * Echoing the article: implement coding standards. Even better, implement architectural standards. Makes review comments less personal and more “hey, follow the standard! no more than three levels of inheritance!” for example.
    * A review checklist maximizes the effectiveness of a review. Don’t have a checklist? Borrow someone else’s in the meantime (http://blog.fogcreek.com/increase-defect-detection-with-our-code-review-checklist-example/)

    Reply

  5. Use literate programming. Most people are working on their own problem when they are called in to review unrelated code. Prior to the review distribute a one or two paragraph description of WHY the code is written
    (not HOW) so people can understand what problem is being solved. This changes the focus from the coding
    standards and variable initialization to a focus on the problem solution.

    Insist that these paragraphs be included with the code in the repository. People who later have to change the
    code can understand WHY the code was written, including special case information that might not be obvious.

    Hire an English major to arrange the code into a book so new people can read it. You can use it to test new hires.
    You can also preserve the program once the original authors “graduate”.

    Reply

  6. Good article! As a relatively senior developer who frequently performs and requires code reviews, I have to fight (I don’t often win!) a few personal pitfalls when performing code reviews for others:
    If a review is very large, it dramatically increases the chance I won’t do a great job. Usually it starts well but then fatigue makes it easier to miss things towards the end of the review. I think we probably don’t have the best culture of expecting developers to spend significant time on others’ tasks, even if significant time is required, and we also don’t have the best culture of trying to keep changes as small or isolated as possible. Most of it I attribute to my lack of attention span, though.
    I am biased in that I often won’t look as hard at the code of somebody I like or trust. That leads me to miss things a lot. If I don’t trust a developer (because he or she is new or because I just don’t trust them based on other biases) I will review a lot “harder” than otherwise. In those cases where trust is lacking that probably also leads me to be less helpful than I could be, I’m probably much more directly critical.
    We use an online code review tool, which makes it easy to do reviews on your own time. I often find that this leads to times when I may be bored with what I’m working on and so will provide a very thorough review as well as times when I have too many reviews on my plate and will delay doing them or do them but not do a great job.

    Reply

  8. Code reviews are a fixed part of our development process. They are conducted in ad hoc style. We read through the pull request in GitHub and search for defects and code smells.

    I think it makes sense to do so. However, I also see a problem with this approach. Once the feature is fully implemented it is not really possible to restructure the whole thing as that would be annoying for the author and tedious for the reviewer. It would be better to have little feedback loops right from the beginning of the implementation. Here things could be changed without the author being too much emotionally attached with his ideas. The extreme form of it would be pair programming.

    Thank you for your tips!

    Reply

    • > The extreme form of it would be pair programming.

      I have never tried pair-programming (at least not consistently) but that’s not to say that it is not effective. Any time a developer gets his or her code reviewed by another person, in the form of pair-programming or code reviews, the quality of the software improves… Reviews improves software quality 🙂

      Pair-programming (and I agree with your comments) is getting feedback on steroids. If the developer is fairly new to the project, pair-programming ensures that the code he or she writes is consistent with the rest of the project and has a good design practices. In practice, I have found it difficult to spend long hours with someone. Fatigue etc. comes into play and after couple of hours… I want to go away and do my own things (I’m aware that in pair-programming, the reviewer and the developer can switch roles and that might just work but I have yet to try it).

      Code reviews (if done correctly) can bring a lot of benefits of pair-programming. Often times, code reviews are performed with more than 1 reviewer. Code reviews scale better than pair-programming. They also help when your developers are in a very different time-zone.

      Here’s a good article on many different types of reviews: http://www.processimpact.com/articles/two_eyes.html

      Reply

Leave a reply to umermansoor Cancel reply