Partager via

Code reviews are too late, design reviews are better

  • Article

I feel that code reviews that happen when coding is finished are too late. It is difficult to tell a person after he or she wrote a decent chunk of code (maybe thousands of lines) that the code is not good and has to be rewritten. Will you have guts to reject the code? What if deadline is looming? Will you recommend to management to cut the feature then? Are you sure you will be understood?

I believe that design review should precede coding. If you are employing TDD, design review should happen before you write your first test. Design review ensures that appropriate architecture, alrgorithms and tools are selected. It should include review of the coding schedule and review of the initial set of tests. Typically many people participate in design review, not just author of the code and the reviewer. This significantly decreases chances of discovery of a major hole late in the cycle. You may guess that design review is particularly important if feature is to be implemted by a relatively junior developer.

If you chose correct design, you may not have to perform final code review at all or perform just a quick sanity check. After all, you know that design is correct and you understand it well (you did participate in the design review, did you?) so you only have to review if the code actually implements the design and check for stupid mistakes. Good example from my team is solution for the infamous problem in the VS HTML editor. It took approximately 8 (eight) developer-months to design and implement the solution that you see in Whidbey. However, only 3 monts were spent in coding, most of the time was spent in design reviews. Interesting that the final code is not large at all. In fact, it is shorter than the previous version.

XP programming supposedly eliminates formal code reviews. However, it does not necessarily guarantee good design. After all, the design will be only as good as the best developer in the pair.

Comments

  • Anonymous
    June 12, 2004
    Hi,

    I think you are right. But I don't really understand what you mean about final code? When a class is finnished? a module? or a method? Beacuse finnal review for me is before releasing a Build and I don't think there is to much code then. (a build of a module). So I can't see the problem with final review. The only problem with final review is if this final review os over the whole application.

    //Johan

  • Anonymous
    June 12, 2004
    "XP programming supposedly eliminates formal code reviews. However, it does not necessarily guarantee good design. After all, the design will be only as good as the best developer in the pair."

    The same could be said of "formal" code reviews - they're only as good as the best reviewer. Remember that XP pairs rotate (daily, usually) so many pairs of eyes will get to see the code.

    Jim

  • Anonymous
    June 12, 2004
    > so many pairs of eyes will get to see the code <

    That's one of the biggest fallacies in software engineering today. Try to find some solid facts on whether this is true... you won't find any.

    As for formal reviews, they are better than the "best developer" and have been proven so in several real scientific studies.

  • Anonymous
    June 13, 2004
    'Final' review is when feature is complete. You may or may not want to perform reviews in a middle of the feature writing, it depends. Sometimes you don't want to review partial implementation since it is going to be refactored anyway.

  • Anonymous
    June 13, 2004
    >>so many pairs of eyes will get to see the code <

    >That's one of the biggest fallacies in software >engineering today. Try to find some solid facts on >whether this is true... you won't find any.

    Putting the rhetoric to one side, why would I need a study to prove that many eyes have seen my code? I know it to be true because I work on a project which rotates its developers constantly.

    I think what you're really saying is that constant reviews by many people have not yet been proven to be better than "formal", post-hoc reviews. Even if that were so, that would not be a reason to dismiss pair programming (which I suspect was the subtext of your comment), rather it is a challenge for someone to prove it. If you're interested in studies defending pair programming and constant reviews, you could have a look at this one:

    http://collaboration.csc.ncsu.edu/laurie/Papers/XPSardinia.PDF

    I would think it hard to argue that identifying problems earlier was less useful and less cost-effective than later, given that the earlier a problem is identified, the cheaper it is to fix.

    There is also the argument that, even if formal reviews are in themselves a good thing, people generally don't like doing them and rarely do them well.

    Jim

  • Anonymous
    June 29, 2006
    PingBack from http://sdesmedt.wordpress.com/2006/06/29/implementing-peer-code-reviewing/

  • Anonymous
    May 31, 2009
    PingBack from http://outdoorceilingfansite.info/story.php?id=22175

  • Anonymous
    May 31, 2009
    PingBack from http://outdoorceilingfansite.info/story.php?id=4539

  • Anonymous
    May 31, 2009
    PingBack from http://woodtvstand.info/story.php?id=11088

  • Anonymous
    June 08, 2009
    PingBack from http://insomniacuresite.info/story.php?id=8050