33

The team in my new company does not have a code review process.

I come from companies with code review as a must culture and thus I don't feel comfortable committing my code without having it reviewed by someone.

I'm a firm believer that code review is a way to improve quality and save time because it catches potential problems earlier (note I am not talking about pair programming though).

  • How can I show that code review is not a time waster but a time saver?
  • Can code review be skipped if you have unit tests?
12
  • off-site resource recommendations are explicitly off-topic per help center. See meta.programmers.stackexchange.com/questions/6483/…
    – gnat
    Nov 5, 2014 at 7:41
  • 1
    Consider asking it here: meta.codereview.stackexchange.com And in my mind, this question can be asked here because he/she just wanna know something concept related to Programming.
    – xqMogvKW
    Nov 5, 2014 at 7:55
  • 1
    Although I'm also a firm believer in code review, I've also had some bad experiences where code review turned into a perpetual war, code review tool (gerrit) turned into a bad avatar of some discussion board with over-passionate debates led by oversized egos. I'm not sure if this is bound to happen in any company, or if this is just a matter of people's maturity.
    – Joel
    Nov 5, 2014 at 10:36
  • Retitled to what you are asking asking because " Is Code Review a must?" is a too broad, unanswerable question as it will depend on a huge number of factors - company size, # developers, revenue, etc, etc. I would mull over how you can both incorporate and market your desire and enthusiasm for good programming practices and software craftmanship on your public sites (resume, linkIn, github, twitter, etc). publish what you care about and what you seek so that the folks you want to be with will see it. This is 'future' advice of course, hence a comment :) Nov 5, 2014 at 12:04
  • 3
    I can't see how this is an "off-site resource recommendation". That doesn't sound like the right close reason to me.
    – alexia
    Nov 5, 2014 at 20:56

10 Answers 10

33

Can code review be skipped if you have unit tests?

But why?

The primary role of peer review is not to catch out bugs.

Yes, you may identify some potential bugs and doubtful, bug-prone code, this often happens, but occasionally spotting some blunders doesn't mean that peer review is a reliable way of ruling out the presence of bugs. Far from that. It's not the right tool to verify the functional correctness of implementation.

Code review enforces code maintainability, though. I will demand that code is clean and understandable (not just for its author) before it goes into production.

The presence of unit tests is completely orthogonal to that. You can have 100% code coverage and all tests passing for totally incomprehensible code.

Code review also serves to familiarize other developers with your work so that they know what is what and are able to pick up from there, or handle bug reports while you're on holidays etc. Knowing what you've done straight away may help them do their job well - keep the codebase consistent (stick to similar patterns and conventions throughout the app), or avoid code duplication.

In broader scheme of things, one also learns and grows as a developer from reading other people's code.

Unit tests can hardly be a substitution for any of it. Yes, if they're well written, they read like documentation, and we should strive for this. But again this is not mutually exclusive with performing peer review, quite the contrary - all the advantages of peer review still hold true, the fact that your peers have some nice unit tests to look at will only make the reviewing process easier and even more beneficial rather than redundant.

3
  • 4
    I don't believe unit tests are a replacement for code reviews, too. But the primary role of unit tests is not to catch bugs, either. Yes, you may identify some potential bugs when writing a unit test, but unit tests are for making sure you won't introduce bugs later when you have to change something. So the goal of unit tests is to keep your code maintainable, too.
    – Doc Brown
    Nov 5, 2014 at 12:18
  • 2
    @DocBrown true that. However catching regression bugs falls under the category of catching bugs, too. Obviously this is one of the advantages that unit tests have over peer review (in this aspect), because they're not a one-off operation. Peer review doesn't even attempt to tackle this important aspect, since it's not feasible to re-review the whole codebase after every single change. Nov 5, 2014 at 12:34
  • 1
    @DocBrown Peer review doesn't even attempt to tackle this important aspect - well, it does, sort of. To an extent. I often find myself pointing out eg. "partner, you're effectively repeating the same logic here, and that's a ticking bomb. One day it's going to change in that other place and we'll forget to update it here..." Nov 5, 2014 at 12:42
24

Are there any studies and statistics showing code review is not a time waster but a time saver?

I don't know of any. It's also hard to conduct such studies, because you'd need two teams that have a task of equal and realistic complexity to accomplish, where one uses code reviews and the other one doesn't. You'd probably need to have them solve the same problem, which means throwing lots of money out the window. And you'd need to repeat the experiment often enough to get statistical relevance, which would increase that money throwing by orders of magnitude.

If you just measure the efficiency of companies using code reviews against companies that don't, it's not only unclear how to measure efficiency, but also what the actual cause is. Code reviews are part of a larger culture. Which part of it actually makes the team more efficient is hard to tell (and may very well depend on the nature of the team or the project). Or the presence of this culture may simply mean that the company is either smaller or younger, each of which has many effects. Or it may well just be that the willingness to submit to code reviews precludes or fosters a healthy distance to your ego ;)

But don't forget: you have your own experience to draw from. It's part of why they hired you. So if you truly believe you can increase efficiency (and your team actually suffers a lack thereof), then communicate that clearly.

Can code review be skipped if you have unit tests?

Nope. If you believe in the importance of tests, then actually your tests should be the first thing to be reviewed. What if they are nonsense? Or if coverage is lousy? Or if they test implementation rather than behavior?

5
  • 2
    I think you made a real good point on code review for test cases. thank you!
    – jparkcool
    Nov 5, 2014 at 8:04
  • 4
    +1 for "you have your own experience to draw from" - actually, if one has really worked with code reviews for some time, he must have seen how many quality issues typically were fixed during a typical code review, and how much knowledge transfer was achieved. From that experience it should be hard not to have a handful of arguments for or against code reviews.
    – Doc Brown
    Nov 5, 2014 at 8:20
  • 2
    Regarding your first paragraph: It would require not only two teams performing the same task, but two teams of equal capabilities, individual experience is going to mess with any attempt to study this. Nov 5, 2014 at 15:38
  • "What if they are nonsense? Or if coverage is lousy?" Or just say return true;.
    – Burhan Ali
    Nov 6, 2014 at 0:30
  • 1
    Read chapter 20 of Code Complete for a thorough treatment of code reviews, and the studies and statistics that support their use. Here are a couple of good summaries: Jeff Atwood's blog and another guy Nov 6, 2014 at 18:57
5

Taken from some random slides I found, but the hard data comes from Steve McConnell's Code Complete book:

Are Code Reviews Useful?

"I believe that peer code reviews are the single biggest thing you can do to improve your code"

Jeff Atwood of Coding Horror at http://www.codinghorror.com/blog/2006/01/code-reviews-just-do-it.html


"Individual inspections typically catch about 60 percent of defects, which is higher than other techniques except prototyping and high-volume beta testing."

Steve McConnell, Code Complete 2nd Edition, page 485

That 60% figure comes from the IEEE paper by Shull et al 2002 What We Have Learned About Fighting Defects which contains the section titled:

"Peer reviews catch 60% of the defects"

1
  • 1
    I think the issue with this is in 2006 we hadn't yet fully embraced pair programming yet, which I feel like has become a sort of replacement to doing peer code reviews in a way. I realize that they aren't comparable in some ways. Nov 5, 2014 at 22:20
3

Disclaimer: This answer is my personal experience :)

I made very bad experiences with code reviews in code that we maintain. Because we usually have only one liners or so and there is not much to review.

But in actual projects I made good experiences, in my examination time my trainer reviewed my code regulary and it helped me a lot to find some mistakes I made very often, that I don't do anymore.

So I would say it highly depends on what are you doing, how many people you are etc.

Also the risk that your codereviews end in a war is not to underrate.

3

You could ask your team leader and/or colleague to peer-review your code, even if code reviews aren't done normally, perhaps as part of your training.

Make sure that your code is well-written and tested before the review.

When I was a team leader I used to do code reviews myself of new employees, until (after a while) I would stop finding bugs or anything else to criticize in their code, and which point I would stop doing code reviews with them; that would happen when:

  • They learned the systems they interfacing with and didn't need my explanations of it
  • They learned to design and/or test their code until it was bug-free before I saw it
  • They learned enough about my coding style guidelines that I'd consider their code maintainable

Code reviews have several purposes:

  • Finding defects in the code
  • Knowledge transfer between team members

I think it's fine to do code reviews of new employees, even if the team chooses to skip code reviews among the experienced team members.

2

There is no thumb rule for code reviews to be done on any software developed...it all depends on the scope of the application, client size and company size. For example if you are building an application where its a simple application where there may not be any further versions are being implemented in future, unit testing is sufficient there. But again code review comes into place when you talk about performance of the application where you need to review the code for any short falling of the code which could have been done in a better way to facilitate faster performance.

Code reviews are usually done where there is a team of more than 2 developers and a tech lead where tech lead wants to ensure the quality of the application and make sure the code standards are followed to scale the application for future enhancements and upgrade it for different upcoming versions.

For example we do have many CMS open source platforms now and these platforms release upgrades from time to time to enhance the platform features, imagine a developer using one of these platforms and has not followed the code standards like hard coding in core files, writing application code in template files, and if this code goes to production and later when client wants to upgrade the platform to new version it will never be upgraded unless the coding is redone as per the code standards for that platform. Here it becomes a serious issue for releasing the code to production without code review being done.

So i would say having code reviews done before release are a must for any professional software companies and the exceptions can only be for personal/very small scale applications where the developer is very seasoned programmer and carries experience with him.

1

Code reviews have advantages that don't come from the reviewing process itself: There is always a dilemma to get code that is of high quality, but created in a short time. Without code reviews, you're on your own, so you might sacrifice quality for doing the code in a short time. With code reviews, there is this reviewer who doesn't let you get away with low code quality - which is exactly what you want, being forced to spend the time to get quality code which is what you wanted in the first place, and which you know will end up saving time because every hour spent on writing better code is two hours saved on debugging (or more).

Without code reviews, you are on your own, so it's up to you to maintain high code quality. A simple solution is to review every change you make yourself and fix things that are not up to your quality standards.

This also avoids horrible situations where code reviews lead to clashes of egos - the situation where programmer A would use method X, while B would use method Y, so if A writes the code he uses method X, the reviewer B insists on method Y, so A rewrites the code using method Y, while if B had written the code and A reviewed it the exact opposite would have happened.

0

If you're an advocate of code reviews, I'm afraid there's no real substitute. The unfortunate and stereotypical case is a workplace which doesn't do code reviews because (A) they're not familiar with the practice, and/or (B) they don't want to devote the time and effort to getting a code review system in place.

Basically, to get what you want here, you need a workplace culture change - and that's never simple or easy. Don't forget that even if your workplace is 100% persuaded that code reviews are excellent and they want to adopt them, actually shifting into the new way of working will still require a significant investment of time, energy and productivity. This investment should pay itself back - but you need to have buy-in for the investment, not just for the payoff. See Roy Osherove's video "Unit Testing and TDD - How To Make It Happen" - the challenges of adopting code reviews are very very similar to those of adopting unit testing.

In the meantime, do what you can to get as much as you're able:

  • If there are other developers who see the value of code reviews, try reviewing each other, even informally.
  • If you have a mentor or some developer responsible for your training, explain to him the value you see in code reviews, and ask if they'd be willing to review your code, at least on occasion.
  • Tell your manager you'd like to review other people's code, because it'll help you understand the system better.
  • If at some point you become a team lead, you can instate code reviews locally, just for your team.

A major benefit of any of these is, if you're able to maintain them over time, then developers around you will start noticing the code reviews. You'll effectively be demonstrating how code reviews can be integrated within the existing culture - and that opens the way for the culture to begin to change. Code reviews help, so if you're able to demonstrate that at a small scale, that will open the way for others - and the culture as a whole - to follow your example.

-2

If your new employer doesn't like the idea of code reviews, it may be because they have a negative association with old-fashioned command and control type methodologies and they're aiming for a more modern, agile type set of practices. In this case, they may be more open to the idea of pair programing, which provides many of the same benefits, and is widely regarded as a more dynamic, modern practice.

-3

Stop worrying about it - your new employer just doesn't care about code reviews. Learn to have some confidence in your own abilities without someone else telling you it's OK to check in the code you've written. You'll soon learn to live without the mind-numbingly tedious process that is reviewing other people's code.

Follow the style guidelines (or just the style) that everybody else uses. Use your experience to decide what needs commenting, what naming conventions to use and so on.

Then test everything before you check it in. The most important thing is that it works correctly.

4
  • 2
    -1: The fact that the OP's new team does not do code reviews does not make it a bad idea to do so. It is a sign of a good engineer to help improve the quality of the development process. Nov 5, 2014 at 15:30
  • 1
    @JørgenFogh I am also in support of code reviews, but you seem to be assuming code reviews would help this specific development process. In addition to this answer, I would ask why they don't do code reviews - they may have a good reason. Perhaps, as this answer suggests - this company hires people that don't need to have their code looked over, or at least the benefits from doing so are just not worth the extra cost. If the OP tries but doesn't have any luck changing anything, this will be the answer to fall back on. Nov 5, 2014 at 16:10
  • 1
    It is possible that the benefits are not worth the cost. However, the fact that the team does not perform code reviews tells us nothing about whether or not they ought to. Nov 5, 2014 at 16:37
  • 4
    -1: "The most important thing is that it works correctly." This is a pretty short-sighted view of what's important when it comes to production code. Code is read more often than it is written. The value of a (well performed) code review goes way beyond checking correctness. Among many advantages, code reviews ensure that code makes sense to someone who didn't write it.
    – Dancrumb
    Nov 5, 2014 at 18:23

Not the answer you're looking for? Browse other questions tagged or ask your own question.