Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I've been wondering about code reviews recently, based on how many questions I've seen on here talking about them.

Between my internships, contracts, and full time jobs, I've worked for six different companies, and only one of them has done code reviews.

At the last two places I've worked, code was good (or at least good enough) if all the unit tests passed, and it got the stamp of approval from our tester. We were all compotent programmers, so if the code worked, we were happy.

Now, there were a handful of times where something came up months or years down the line that might have been caught in a review, but they were so few and far between that I'm not sure it'd be useful.

I guess my question is:

  1. Do you consider reviews to be useful?
  2. How often do you do them?
  3. How much of your time is spent reviewing code?
share|improve this question

closed as not constructive by Jarrod Roberson, Yannis Rizos Feb 24 '12 at 21:03

As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, visit the help center for guidance.If this question can be reworded to fit the rules in the help center, please edit the question.

2  
@YannisRizos Would this be more on topic if it was asking what the benefit of code reviews is? Or why code-reviews are so important? –  Rachel Feb 24 '12 at 21:08
1  
I didn't realize programmers.stackexchange was as strict about this as stackoverflow. It seems like many of the questions here are opinion-based. –  Matt Grande Feb 24 '12 at 21:12
2  
@MattGrande We welcome some subjective questions. Your question has multiple problems though, the most obvious being that you ask three distinct questions. Furthermore, the wording doesn't help, there is nothing in the question text that invites the sharing of experiences over opinions... –  Yannis Rizos Feb 24 '12 at 21:18
1  
"there is nothing in the question text that invites the sharing of experiences over opinions" I really think that's a lame excuse. I specifically ask how often people do them, how much of their time, etc. I think it's clear I'm placing value in peoples experiences. I also don't think this was a duplicate. There were, granted, similar questions, but I didn't see any that questioned if reviews are valuable. –  Matt Grande Feb 24 '12 at 21:23
4  
I think the key question is whether the ANSWERS to this question, plus any subsequent answers that would be posted if the question were re-opened, would be USEFUL to future seekers of knowledge. That is, if tomorrow, somebody searches for "benefits of code review" or "reasons to code review" or something similar; will that person benefit from these answers. Having read some of the answers below, I think YES, a future searcher would benefit from the presence of this question. I don't have the rep to vote to re-open this question, but if I DID, I certainly WOULD. –  David Wallace Feb 25 '12 at 0:10
show 4 more comments

7 Answers

  1. Absolutely. There are lots of bugs you can catch early via code review. And there are certain types of bugs you can hardly catch any way else than code review (e.g. some concurrency bugs). But what is even more important: (group) code reviews are a great way to spread and share a coherent, consistent view of design, architecture, coding idioms and best practices throughout the development team. The only better way I can imagine is pair programming - which in fact is an extreme form of code review.
    The prime goal of (classic, formal) code reviews is in fact not catching bugs already present in the code, but preventing (future) bugs from getting into the code, via the above mentioned continuous communication / mentoring / discussion between team members, generated during review meetings.
    This is probably one of the reasons why code reviews are still rare. They show their real benefit only in the long term, so it takes perseverance to get there, and most managers (and developers alike) have never experienced it.

  2. In our current project, we have been doing it only occasionally up to now, but it is bound to change. The new aim is to get every code change reviewed by someone else before it gets into production. In an earlier project, we did that and it was useful. In an even earlier project, we did formal group code reviews, and it was still useful, but fairly slow and time consuming. We couldn't have reviewed every code change in that manner without slowing down our pace of development enormously. But for some types of projects, even that pays off. If human lives are at stake, your prime motivation isn't saving dimes.

  3. Up to now it was no more than an hour per week on average. According to our new process, this is surely going to increase. In the earlier projects mentioned, IIRC it was 2-4 hours per week on average.

share|improve this answer
add comment

Let's think about the best case scenario: the code is perfect (which will never happen)

Would code reviews be useless? No, everyone involved in the review would learn something and the review would facilitate conversation and exchange of ideas.

Anything less than the best case scenario has been addressed by other answers.

As far as how much time should be spent reviewing and how the review should be structured...this is highly dependent on many departmental/organizational factors and cannot be answered without this context.

share|improve this answer
add comment

You don't mention whether your coders got the code right on the first try, or if they commited code where QA found bugs that required fixing. If QA is finding bugs, perhaps doing code reviews would lower the number of bugs they find. If QA is finding less of the more obvious bugs, they have more time to find the less-obvious bugs. And the more time they spend finding the less-obvious bugs, the less likely your customers will find those same bugs.

share|improve this answer
    
That's a great point. –  Matt Grande Feb 24 '12 at 21:01
add comment
  1. Yes. Much can be learned from reviewing code, even if it's correct and simply written poorly.
  2. As often as time allows, usually daily. At least a few times per week is enough.
  3. Usually 30 minutes to an hour, depending on the nature of the review.

Remember, just because every company doesn't conduct code reviews doesn't mean it's a waste of time to do so. Perhaps they don't all simply realize the benefits or even know to include them as part of their development process.

share|improve this answer
1  
Oh definately, they could be extremely useful, but I've just never had that much exposure to them. Not every company unit tests, and I think that's pretty important! –  Matt Grande Feb 24 '12 at 20:46
add comment

1 - I do consider reviews useful, even if it's an informal process. Unit tests passing doesn't mean you caught all the cases, and unit tests attempt to test code in isolation, so you might have written code that ignored something that may cause problems when your code is integrated with other components. Plus, it might help you improve your ability and style if people can suggest better ways to write what you have written.

2 - I do them if I'm the reviewer for other people's fixes, though in my last company, we used Review Board and I would be requested frequently to look at people's code. I also sometimes glance at major code changes to see if the changes are sane.

3 - Perhaps 5% of my total time.

I always try to get someone to look at my code. As competent as you may be, it's good to have other people you trust to look at your code and see if they can offer up suggestions or give you the thumbs up.

And here's the link to Review Board, which can hook in with source control systems like svn and git.

share|improve this answer
add comment

In some shops pair-programming has taken the place of code reviews. I suppose you can think of pair programming as a process of continuous code review.

In the humanities there is a famous saying that "Even Homer nods". Meaning that even if you are the worlds greatest coder, sometimes you are going to be distracted, off your game, or just plain screwing up. A second set of eyes is incredibly helpful in catching stuff like this. The earlier bugs or 'mis-features' are caught, the cheaper they are to fix. Sometimes these mistakes will pass existing tests, but be laying like booby-traps for when the code needs to be extended.

It also has the side effect of keeping everybody in the team at least a little aware of how the other bits work. This help avoid disasters where the one person who knew how the accounts receivable code worked is hit by a bus or leaves the team.

share|improve this answer
    
Well, I don't see what The Simpson's has to do with anything. :) –  Matt Grande Feb 24 '12 at 21:02
add comment

I consider code reviews very useful and helpful. Even if by some miracle the code is bug free it opens up opportunities to valuable insight. I like to see how to do things differently and/or sometimes better. When I do reviews I may also pick up something new or better while looking at the code someone else wrote.

How often they are done depends on what we are changing. If it is a bugfix then it is likely it will go straight to QA after the developer submits a merge request. If it is something larger and time permits then the code will be reviewed.

The amount of time spent reviewing depends on the code and the developer. Some developers are more adept than others and some code changes are bigger than others so it varies. It never goes over an hour for me...plus the meeting will have about it (~30 minutes).

share|improve this answer
add comment

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