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 worked at two companies, who each had a different methodology when it came to code reviews:

In the first company, a code review was conducted by the team leaders and was required after the completion of every module.

However, in the second company, team leaders weren't required to conduct any code reviews, and just checked for functionality and design issues.

So I am confused. Is the code review process really needed? If it is, why? And if it isn't, why not?

share|improve this question
25  
If senior developers tell junior developers to do things in a certain way, there is frequently a very good reason.... –  user1249 Oct 22 '10 at 8:42
2  
@Tilsan The Fighter: It's good that you're asking questions - inquisitive is, or should be, a programmer's feat - but please make them understandable and easy to read. –  gablin Oct 22 '10 at 9:01
7  
@Thorbjorn - That's only true if the senior developers are senior because of skill and not because of duration. I've seen plenty of bad code and design by senior engineers –  KallDrexx Apr 26 '11 at 19:37
6  
There might be a good reason, and it's good to find out that reason, but you shouldn't trust their advice blindly just because of their title and X years experience. I've asked the senior engineer here why he made a lot of bad code and all I've gotten was a shrug with a "I don't know". There are enough bad engineers out there to make me realize not to put any trust in just a title. –  KallDrexx Apr 26 '11 at 19:49
3  
+1 KallDrexx - that's what I've found too. I've often had a "senior" developer not know why you shouldn't just stick everything into a static class, or know anything about testing, or know any proper design patterns.. "Senior" typically refers to tenure and not skill from what I can tell. –  Wayne M Aug 16 '11 at 14:15
show 2 more comments

16 Answers 16

I personally think that every piece of code should go through a code review, it doesn't matter if you are junior or senior developer.

Why? For starters your title doesn't state anything about how you develop, and a senior developer could learn something from the junior. At our company we shift around so one of the other members of the team review your code...mostly we are teamed a "junior" and a senior together, so all the stuff that doesn't get said on a daily basis can be caught in a follow up. If the senior doesn't like the junior code he should listen to why the junior did as he did and look at it and see if that's a feasible solution that might be used in the future...it's a matter of getting wiser no matter who you are.

One important thing about code review is not being too nice, if you are being a nice guy you'll just allow more and more messy code to evolve in the system. Just as yesterday I started reworking a complete application that a former employed juniordeveloper wrote, and my god that code could have needed a review before he left.

I don't see why it should be the teamleader only doing reviews but it requires a person that's not affraid of picking a "fight" over a piece of poorly developed code, and it has to be a person that cares about how the code should be. Not all companies hire people that actually care about what they do, and those bad eggs should IMO not be allowed to do code reviews as they are likely just to shrug their shoulders and say "OK" to bad code.

share|improve this answer
7  
Actaully, there is a point in not only having the teamleader doing code review. Even a less experienced developer should be able to follow the code. :) –  Guffa Oct 22 '10 at 9:01
56  
Team leads should have their code reviewed too, as often junior developers can learn valuable techniques or at least see how "good" code is supposed to look. And just because you're a team lead doesn't mean you can't make a mistake. –  TMN Oct 22 '10 at 13:19
3  
@TMN, I can't agree more. Team leads aren't always picked because of their skills or experience; sometimes they're all that's left after a mass exodus (which has happened several times where I work), or a large layoff. Everyone's code should undergo review, regardless of experience, status, or title. –  bedwyr Oct 22 '10 at 15:49
2  
@TMN Too right. The title "senior developer" does not mean "incapable of making mistakes" or "incapable of improving". If it does, that's not the kind of senior developer I want on my team. –  Brandon DuRette Oct 25 '10 at 17:49
2  
I'm very experienced and good at what I do. I make mistakes, many of which have been caught by code review. –  David Thornley Apr 26 '11 at 17:46
show 4 more comments

Basically code review is needed for all programmers regardless of experience. It is the Quality Control of software development, and one of the reasons Open Source can be of very high quality.

EDIT: The reason is that a code reviewer today is in exactly the same role as a maintainer later. If the code doesn't make sense to him today, it will not make sense later either, making bugs more expensive to fix. Hence, MAKE it understandable today while the developer still remembers the code. Also the reviewer may see bugs or omissions that the developer missed.

Unfortunately very few wants to do it, but seen from a business point of view it should be mandatory.

share|improve this answer
2  
you might be interested in this code review proposal. It would be nice if we can get the ball rolling on this. –  greatwolf Jan 13 '11 at 10:46
show 4 more comments

I work in a place where code review is now a requirement but was not as little as 3 years ago. It has made a huge improvement in in our code and in the ability of others to maintain the code later. Even senior, very experienced developers make mistakes which can easily and quietly be fixed in code review before QA finds them or worse before the customer finds them. Further at least one person other than the orginal developer is familar with the code.

Often when an organization tries something new to it, as we did with code review, there is much resistance to change. I have seen almost none of that (we were estatic to get a formal QA department too.) with code review. It pretty much takes only one or two reviews to see the value.

I have found new techniques I had not considered both in doing a code review of someone else's work or in having my code reviewed. We have found competence problems with new hires relatively quickly by having code reviews and more importantly by how they responded to the code review. We have learned what things that seem perfectly clear right now in the thick of programming that section that will not be clear in maintenance. This is invaluable. It may be that the only thing needed is a comment as to why something was done. We have found some fundamental misunderstandings about our database design that needed to be corrected for a report to actually have the correct information.

Often what I have seen in a code review is that, by the very act of explaining something to someone else, the developer will have a lightbulb turn on in his head and realize that there is a bug the reviewer didn't see.

And boy can code review identify those cowboy programmers who won't follow any standard or use mandated tools and whose code will be nigh onto unmaintainable by anyone else. And it can force them to get with the program or get out too.

The people most resistant to code review are often the people the organization most needs to get rid of because they know in their hearts their code can't pass a code review.

share|improve this answer
2  
"We have found competence problems with new hires relatively quickly by having code reviews and more importantly by how they responded to the code review" - This is a very important benefit (and I agree, how they respond tells more than the mistakes they make). –  Stephen C. Steel Oct 26 '10 at 3:04
1  
+1 for capturing the why's –  user1249 Dec 5 '10 at 12:43
1  
+1 for "identify those cowboy programmers" –  Dave Jun 25 '12 at 21:08
add comment

Agile guy would say: "You don't need a code review, just do pair programming and write good tests" :)

share|improve this answer
9  
And switch pairs often and recognize that the team, not any individual, owns the code. –  Don Roby Oct 22 '10 at 15:29
15  
Agile guy is wrong. Code should be reviewed by a mind which is independent to the mind(s) which created it. (It's very easy for a pair of programmers to talk each other down a blind alley without realising it!) –  Peter Boughton Oct 22 '10 at 20:36
5  
Pair programming is continuous code review. –  user1249 Oct 26 '10 at 6:17
3  
@Peter: Agile guy also re-pairs promiscuously, and practices common code ownership, so over a span of time (days to weeks), most of the rest of the team have reviewed the code the original pair wrote. Agile guy has an answer to everything. Some people think Agile guy is a total smartass. –  Tom Anderson Oct 26 '10 at 18:04
2  
Pair programming fixes the main problem with code reviews -- they occur much too late. I hate going to code review to see that the developer has spent a week redeveloping a library algorithm or data structure. –  kevin cline Aug 16 '11 at 18:28
show 3 more comments

Code review is a good way of spreading knowlege and good practice through a team. In my experience it is a good idea to make sure all code gets reviewed, and try to vary who reviews what code.

In my current team everyones' code is reviewed equally, and both programmer and reviewer have to be satisifed with the code before it can be released. This applies just as much to senior developers being reviewed by more junior developers, a junior developer reviewing another, or other senior developers reviewing each other. If the reviewer is inexperienced or does not feel comfortable reviewing any particular piece of code then they will team up with another developer (and maybe also the original developer) to do the review as a group.

share|improve this answer
2  
Yep, code review is as much quality control as it is knowledge sharing. Everybody can learn from a good code review. The reviwer and the reviewee. –  Guillaume Oct 22 '10 at 14:46
1  
My company is similar to that of flamingpenguin. Every check-in is reviewed by another developer. Rank has nothing to do with who reviews what. It's a peer-to-peer process. The requirement is that all code is reviewed. Parent-child style code reviews only serve to drive away the 'A' developers, leaving a 'B' team lead reviewing 'C' juniors. The best a parent-child style team can hope for is mediocre code. If they are lucky. –  Jim In Texas Aug 20 '11 at 0:15
add comment

I've been in the business for more than 20 years, working for software companies and for different businesses and none of these places had a code review process. However, I can understand and appreciate the benefits of having one. Essentially, as I understand them, they should be used to ensure adherence to standards, which should be followed in order for others to be able to more easily maintain the code in the future. Code readability might also be checked in a review process as this too will ensure that maintenance can effectively work with the code.

Currently, I work in a small shop where I am the single developer. Occasionally we have brought in contractors to help with the backlog. At least one or two of those contractors have written code that didn't necessarily meet mine or the companies' standards, but it did work well and was at least somewhat understandable. When I pointed this issue out to management, they didn't care, they just wanted to know if it did what we paid them to do. So, I guess it just depends on the company, and whether or not clean, easy to maintain code is the desired product, or if they just want something that works well for the money.

Obviously as a developer, and one who has to maintain code, I would like to work with clean code that follows a standard of some sort, but I dont always have that luxury, so I make the best of it and deal with what I have, even if it means sometimes having to re-write some code in my own standard.

share|improve this answer
add comment

Code reviews can identify defects earlier in the software lifecycle when issues are easier (and cheaper) to fix. At SmartBear we have developed a peer code review tool and have also done a lot of research into how to make code reviews effective. Based on data from our customers, defects found in code review are 8-12X cheaper to find and fix than defects found in QA. The cost savings alone makes code review worth it, but there is more value than just that. Code review is an excellent way for everyone on the team to learn and improve as software developers.

There are some traps that can cause code reviews to become less effective, and it seems like your organization is caught in one of them. Make the code review about the code, not the people. Titles mean nothing in a code review. Appeals to authority (you must do it my way because I'm the team lead) do more harm than good. Teach instead. Senior engineers should explain why it should be done their way, not just demand that it is. If you're having a hard time explaining a concept, it's a learning experience for you too. Both of you will be better developers for the effort you put in.

share|improve this answer
show 2 more comments

I think code reviewing ALL CODE is overkill. The amount of time it takes to review all code could be better spent elsewhere. Alterntivley I think critical code and or particularly complex pieces need code review but certainly not every line of code.

share|improve this answer
7  
I couldn't disagree more. Every line of code should go through at least 2x reviews... the original developer should review absolutely every line-change prior to commiting them, and at least one other developer should perform a peer-review as follow-up. Very rarely does code make it through a review without at least 1 good question being raised; peer reviews also increase awareness between team-members of what--and how--others are completing their assignments. –  STW Oct 22 '10 at 15:47
3  
@Gratzy -- I absolutely swear by it; normally it adds ~%10 to the dev-cycle; a small investment for how much we catch early on. –  STW Oct 22 '10 at 16:01
4  
@Gratzy -- simply because we're not perfect. Our team has grown significantly, and we have some turnover (mostly contractors). In the past when we back off on the review policy problems creep back in nearly immediately. The review process is simply a critical step in maintaining an effective team and producing a quality product. Reviewing all code isn't difficult; especially if you have a couple senior devs who are very good at finding uneeded code. Most duplicate code originates from developers who do well, but just aren't aware of an existing approach. –  STW Oct 22 '10 at 18:36
5  
I'm with STW on this - fixing problems caught during review is much cheaper than trying to debug/maintain code later because it wasn't thought to be critical. Also, code reviews only take time if the code is bad - good code is quick and easy to read! –  Peter Boughton Oct 22 '10 at 20:41
7  
Of course they shouldn't, but that doesn't mean they aren't! (How many teams have perfect developers?) What lines of code do you not review? How do you make the decision whether a particular change in a particular file should be reviewed or not? –  Peter Boughton Oct 22 '10 at 21:22
show 10 more comments
up vote 2 down vote accepted

Code Review: The Code Review process must be a vital one for all, I will explain who are all get benefits due to conduction of code review as well as what are the benefits they are getting.

1. Benefits Obtained By the Company Due to the Conduction of Code Review: If frequent code review is conducted, then the company can get the final products in much better optimized way which help them to get branded name in their market and also help company to get or improve their current CMMI level.

2. Benefits to Team Leader Due to the Conduction of Code Review: As we all know a teacher can easily identify the mistakes, because they are reviewing the answers of their students more frequently, so they can get an idea , in which areas there may be possible for wrong things. similarly the team leader also knows what are the things goes wrong in this areas. How can we rectify those.And also Help the team Leader to grab the news ideas from the junior developer too.

3. Benefits to The Junior Developer Due to the Conduction of Code Review: Junior Developer can easily grab the ideas about Code review Process, also they are able to get what is the coding standard, For Instance: to create API in a proper way, they will learn the standardization of the coding which may help them in future specially when they are become in Higher level post.

So My Conclusion is Code review is a Very Very Vital Process for all[even for team Member too], since code Reviewing helps us to correct our careless mistakes in our code,because we are all human beings, so we cannot predict that we never do careless mistakes in code.

Thanks Tilsan The Fighter.

share|improve this answer
add comment

In my opinion, code that is going to be used by a company, whether it was written by a Junior or Senior developer, should always be reviewed. Why? Because what if the code had several bugs? And what if, during the time this code was being used, the program crashed? To stop these things from happening, all code should be reviewed before use.

But what about those companies that don't review code? They're probably the companies that have lots of tech problems and lots of, as they tell consumers, "crashes" ;-).

So let me answer all of your questions:

  • Yes the Review Process is necessary.
  • Why? Because of the reasons I stated above.
share|improve this answer
add comment

What is the difference with interfering with your ideas before checking in your code(review) or afterwards due to a bug, getting too clever/difficult to understand, or not following accepted standard practices? Is it your ego?

You can't disreguard the merits of code review or anything else just because it is being implemented poorly by a less qualified team member you don't respect. Code Review isn't some highly complex process that only a few super programmers are capable of grasping. I'm not sure there are many programmers or professional writers who are capable or have the time to self-edit.

Have you ever returned to a line of code a few months later and wondered what was I thinking? There would be a better chance of catching it with a code review. You just caught it and you're only slightly better than the programmer you were awhile ago - I hope.

share|improve this answer
add comment

IMO a code review should be essential for all developers, but only when the people doing the review are competent themselves. In the past I've had code rejected in a review because, I kid you not, I followed SOLID, did some Dependency Injection and had the code organized into namespaces and folders according to the logical design, and included a small suite of unit tests to verify the code. The code was rejected as "too complicated" and I was told to use one class that smushed everything together and remove the tests because it wasn't how the company wrote code.

A code review like that is worthless, but a code review with a competent team can often illuminate something about the design (e.g. Why you should do X and Y but not Z) or indicate an actual flaw (e.g. Doing X will cause Y to fail for incorrect reasons).

share|improve this answer
add comment

Of course Code Review isn't needed. Then again, neither are tests, continuous integration, source control, customer involvement, profiling, static analysis, decent hardware, one-click builds, bug tracking, the list goes on.

Along with Code Reviews, the things I mention above are tools that help ensure software quality. With a combination of skill, luck, time and determination; you can deliver quality software without any of this stuff, but it's more likely that you won't.

In your scenario, there is nothing to be confused about. Not every organisation indulges in every best practice. They may disagree with it, it may conflict with a different best practice that they do implement, or they may consider that the overhead of implementing it is too great for them at this point in time. Depending on their circumstances, they may be correct in doing so, or they may be making a false economy. For some tools, (e.g. source control) the payback/effort ratio is so good that using it is a no-brainer; for others it's less clear.

There is no doubt that code review is a practice that introduces a significant overhead. Because of this, organisations will seek to minimise that overhead, either by not doing it at all, or by only doing it in certain situations (e.g. for a junior team member, or a particularly hairy change). It is not always obvious that it pays back more (in catching bugs, reducing technical debt or sharing knowledge) than it costs. Most of that payback is difficult to quantify, whereas its very easy to count the number of man-hours your organisation spends doing reviews. The easiest bit to quantify (reduced bug count) is easy to attribute to other factors (e.g. "of course it has fewer bugs, it's more mature").

share|improve this answer
add comment

They're absolutely necessary since they have little experience.

share|improve this answer
show 1 more comment

We make online football game in Turkey. A lot of users and game masters helps us on functionality. Also they give comments on needed features. So I think, if you have a lot of users, functionality tests can be done for helping or getting badges. Collobration of developers, game masters and users with forums, support teams and private test environments create a social project.

Sure code reviews and sharing experiences between development team is needed but if it's not critical you don't have to force yourself.

share|improve this answer
add comment

I think that how verbose 2nd party inspection is depends on your lifecycle, whether you're more agile, or more waterfall in your processes. I think it's reasonable to do high level designs/inspections, as well as more detail level design inspections. I think it's also good to involve several members of a team to inspect.

share|improve this answer
add comment

protected by Yannis Rizos Apr 2 '13 at 9:06

Thank you for your interest in this question. Because it has attracted low-quality answers, posting an answer now requires 10 reputation on this site.

Would you like to answer one of these unanswered questions instead?

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