56

One of my team members, a junior programmer, has impressive programming skills for his level of experience.

And during code reviews, I believe in emphasizing learning, not pointing out mistakes.

But should junior programmers be involved in code reviews for more senior programmers? Or should code reviews be attended only by programmers with corresponding experience?

11
  • 56
    What's all this "junior" and "senior" stuff all about? IMO, whether or not a programmer is qualified to review other people's code should be determined according to ability and experience - not title....
    – Anthill
    Feb 12, 2013 at 10:04
  • 23
    And Title should normally be determined according to ability and experience. If that junior is good enough to review seniors' code, its time to change their title.
    – superM
    Feb 12, 2013 at 10:07
  • 19
    But sometimes this title is determined by HR politics and games :) Feb 12, 2013 at 10:09
  • 4
    What, exactly, do you mean by "junior programmers"? Are these people with less experience in the application or simply less experience in industry? In my experience, it's possible to have a junior staff member be the most experienced person on a given project because they have worked on it the longest or most recently.
    – Thomas Owens
    Feb 12, 2013 at 10:30
  • 4
    @ThomasOwens, By "junior programmers", I mean the people with less experience in industry. Feb 12, 2013 at 10:37

10 Answers 10

61

The primary purpose of a code review is to find defects or potential problems. The required participants in the review should be the people who are best suited to identify these problems, regardless of their title or seniority.

As an example, if an application is being developed in Python and the junior engineer has more experience with the Python language than the senior engineer who wrote the code, then they might be a valuable asset in pointing out alternative methods of doing something, but they may also have less knowledge of the system as a whole.

Beyond the experience in the tools and technologies, also consider experience in the application domain. Someone with 20 years of experience but only 1 or 2 in the financial industry may be helped by having an overall less experienced developer with only 5 years of experience all in the financial industry review his work.

Inviting less experienced staff members to observe and participate as much as possible the code review process may also be beneficial to allow them to learn a code base, ask questions, and learn about what is expected of them in not only code reviews, but in the code that they produce. However, you probably don't want too many people involved (focusing instead on the people who can fully support the code review and its purpose) in the process.

This really applies to any kind of review - requirements, design, code...

12
  • 4
    +1 for "The required participants in the review should be the people who are best suited to identify these problems, regardless of their title or seniority." And also for excellent answer. Feb 12, 2013 at 12:33
  • 62
    "The primary purpose of a code review is to find defects or potential problems." Completely disagree. The primary purpose of a code-review is knowledge-sharing; the second purpose of a code-review is establishing a coding standard; any bugs found during review is more luck than judgement. programmer.97things.oreilly.com/wiki/index.php/Code_Reviews
    – pdr
    Feb 12, 2013 at 12:37
  • 9
    @pdr A coding standard should be established well before the first line of code is written. If you're using reviews to establish the standard, it's too late. It may be a good time to tailor the coding standard as you're developing - you can use reviews to point out weaknesses or suggest improvements to the standard, but I can't imagine starting a development project without some standard (even if it's just the language's suggested guidelines).
    – Thomas Owens
    Feb 12, 2013 at 12:45
  • 5
    How do you even know what to put in the coding standards before the project begins and it becomes clear (through code review) that different team members approach the same problem in different ways? We're not talking about casing on method names, where there are generally language standards, we're talking about things like NUnit vs MSTest; repository patterns; the ability to say "Hey, I already wrote a wrapper for WCF clients. Take a look at mine, take the best of each and make it a standard." This stuff only comes of code-reviews and is the best reason to do them.
    – pdr
    Feb 12, 2013 at 12:51
  • 4
    Unit-test framework was probably a bad example, but it's common for say, two different developments to require a file to be unzipped. Two different developers may use different libraries because they've used them before. You can't have ALL these discussions up front or you'll get hung-up in more meetings than development. Knowledge sharing, through code-review, is the single most important thing for making sure these problems don't spread.
    – pdr
    Feb 12, 2013 at 13:10
84

Should junior programmers be involved as code reviewers in the projects of senior programmers?

Yes they should. It is a good learning experience to read other peoples' code. (And that applies both for good code and bad. Though one would hope that a senior developer's code wouldn't be bad ...)

Obviously, it is unwise to only have juniors doing the code review. And unwise to put too high expectations on the juniors in terms of what they can find. However, you could also be surprised by the fresh insights that the junior programmers can bring to the table.


Another Answer mentioned juniors being / feeling intimidated. That is NOT what code reviewing should be about ... for either the reviewed or the reviewers. If that is happening, your group needs to change the way it does its code reviews ... and maybe the intimidators need to be pulled into line.

3
  • I think what mouviciel means is that the seniors' code can be intimidating, not the seniors themselves (if that's the case, then yes, the team has more serious problems than who gets to review code).
    – yannis
    Feb 12, 2013 at 10:49
  • 6
    @YannisRizos - 1) I don't read it that way. 2) That's where "it is unwise to expect to much" comes in. If the senior's code is "intimidating", then it especially good for the junior's development to try to read / understand it.
    – Stephen C
    Feb 12, 2013 at 10:52
  • 1
    Learning how the senior programmers think is another valuable part of code reviews for junior developers. When I was a junior developer code made more sense once the senior developer reviewed it with me. Feb 12, 2013 at 19:26
39

I would add that if a "Junior" programmer can not understand a seniors code then that in itself is a good measure of the code. OK there may be times when it just isn't possible to write code that everyone can understand, but hopefully those are exceptions - if only 1 or 2 people can understand the code then what happens when those people are not available and there's a problem with it?

Giving people new challenges helps them develop; it might also be that not everyone is cut out for reviewing code but it seems dogmatic to insist someone has a title (determined by HR politics and games) before they're eligible to help in a review.

As others have pointed out a code review can be a two way process; it helps everyone understand the code base, so shares the knowledge, it helps juniors learn new and better ways and techniques from their seniors and it helps the seniors refine their understanding and by writing to ensure everyone can follow the code you have more eyes that can catch mistakes.

3
  • 6
    Now that's a good opening sentence.
    – pdr
    Feb 12, 2013 at 13:00
  • If the code is using more advanced techniques (e.g. using set operations instead of arrays and loops), then what happens is that someone in the team raises their game. Feb 12, 2013 at 20:25
  • 1
    When doing code reviews it's an extremely strong indicator that the code needs a comment or two if anyone has to ask what a particular piece of code does. Aug 3, 2013 at 19:00
25

The purpose of code reviews is to catch problems that testing can't catch, like maintainability problems and corner cases. I would argue that in many ways junior programmers are better suited for that purpose:

  • They have more time available in general.
  • They are more likely to take it slowly, line by line, out of necessity to understand the code.
  • When you talk about code being maintainable, that means by everyone in the company, not just your top programmers. That means your junior programmers must be able to understand the code in order to declare it maintainable.
  • They are often less likely to make bad assumptions, trusting that something works the way they assume it should work.
  • Their education in a programming language is more recent, and less likely to be muddled together with years of experience in another language. For example, a senior might accidentally use a habit he picked up from C++ that compiles but works subtly different in Java. Juniors pick up on those sorts of errors more easily.
  • Code reviewers only need to identify problems, not necessarily propose a better solution. They will often make comments along the lines of, "I can't really figure out how to do it better, but this part is really confusing because of all the repetition." A more experienced programmer can then easily make the improvements even though they may not have noticed the problem at first.

That's not to say there aren't other ways in which senior programmers are better suited for doing reviews, but my point is you are doing a disservice if you aren't taking full advantage of the diversity of your team.

13

Juniors will often be asked to maintain the code, it is critical that they can understand it.

At times juniors are the only people available to review the code of the senior developers. Should code wait to go to QA (we don't push anything out of dev without a code review and I am assuming this type of code review as well) because the senior's boss is on vacation?

I have also specifically asked juniors to code review something when I knew they would be doing something similar for a different client shortly or if I knew they had worked on something else that was similar or that they had a particular skill set.

If the code is fairly straightforward, I often get a junior person to do the review. Why waste the seniors person's time if the junior person is quite capable of doing the job? If juniors feel intimidated by reviewing senior's code, get them to look at the easier pieces initially. After all you cant get past being junior until you stop feeling intimidated.

I have often found that if I have to explain the code to a junior person who doesn't understand it, I will see an error I made (usually in an assumption) and that no experienced code reviewer would have caught because the code does run but doesn't do exactly what was intended. So just the act of explaining things will often help the developer see an issue without the code reviewer finding it. Since more experienced people are not often taken through the code step by step, these types of things are found more easily when a junior does the review.

I find that having junior involved in reviews has several good effects. First it makes them more confident when they can understand a senior person's code. It makes them even more confident when they can find a bug in that code.

It exposes them to thinking processes outside their own and lets them see other ways of handling things. Even as a senior person, this has happened to me - seeing a different way of solving a problem can be an eye opener to new possibilities.

It helps them learn to read other people's code and it gives them a chance to ask what the code is doing while it is still fresh in the minds of the author. That's a lot better than having to maintain the thing six months later when the author is long gone or is busy on another project and doesn't have time for questions.

It's good for the seniors because the questions both expose potential areas where the junior is weak and needs mentoring (so they can take more responsibility and give the seniors more time to do other types of tasks) or areas where the code is simply not clear to anyone except the author (which means it might not even be clear to the author a year from now when it needs to be changed). It also helps seniors realize that the juniors might be smarter than they have been giving them credit for being. It helps keep everyone on a professional footing. After all if you exclude juniors then you are clearly implying that you don't think they are capable of understanding the code which is psychologically unfortunate.

Juniors reviewing seniors code can generate more professional respect in your organization. Seniors may realize they have been underestimating the juniors and juniors may realize that the seniors do know more than they gave them credit for. Juniors sometimes think they have greater skills than they have. Being exposed to code they can't write is good for these people because they start to realize they have much more to learn. It also will spur the best of them on to get the skills. In school sometimes the B students don't understand why they didn't get an A until someone shows them a sample of the A level of work. Same with juniors to seniors in code review.

7

My answer is: Sometimes. It's going to vary from programmer to programmer, and from task to task.

For:

  • If you want those juniors to learn how to do an effective code review, then the best way is for them to see how the seniors do it.
  • A junior programmer might have more experience than a senior one in a particular language/domain/etc.
  • By forcing juniors to evaluate seniors' code, they are inevitably going to learn things. Pair programming is going to be a more effective way of doing this though, since any questions the junior may have can get immediate answers.
  • Nobody's code is sacred, and nobody is so good that their code shouldn't get reviewed. If you don't do this, who is going to review your top guys' code?
  • Not all juniors are equal, and not all seniors are equal. Sometimes there might not be much of a gap, so don't get hung up on job titles.

Against:

  • There's a risk of the reviews getting bogged down with non-issues from juniors.
  • The level of knowledge/skill required might simply beyond the junior's capabilities. This is going to not only waste their time, but quite possibly demoralize them too.
5

I'm a strong believer that everyone in the team should be involved on both sides of code reviews. Juniors should review Senior code, and vice-versa. Why both? Because usually it's not just about if code is "solves the problem". I can't tell you how many times I've had to explain a piece of code to someone and suddenly come up with a much better way of doing it by the end of the explanation. Code reviews serve probably 3 purposes:

  1. Ensure code is correct
  2. Get the writer to think about how others will see their code
  3. Get the reader's feedback on what could be improved and a general second pair of eyes

I'm a junior and I commonly review senior written code. It's a general company policy "everything gets code reviewed by someone". I learn a lot from these reviewing their code and having the opporunity to ask questions about why things are done in a certain way. And sometimes, I propose a cleaner way to do a certain piece of code and such. Much more rare than people telling me how to improve my code, but it has happened at least once.

It also matters how formal your code reviews are. Ours are very informal and consist of "hey will you look at my code" being said across cubicles or in a private IRC channel. I could imagine if you review the code in a more formal setting, the junior would probably be much more intemidated about reviewing a senior's code.

2

Absolutely, junior engineers should review senior engineers' code, at least some of the time.

In my experience, it's pretty rare that the reviewer in a one-on-one code review actually sees an error that the original coder misses, whether the reviewer is senior or junior; the reviewer doesn't even have to be human. It's very common, on the other hand, that the original coder recognizes a mistake while trying to explain the code, and the more junior the reviewer, the more likely this is, because of the required depth of explanation.

Some more often overlooked benefits of code review, in my opinion, that are possibly more important in the long run than catching errors:

  • Sharing knowledge of what's actually going in the code base - "Wait, I think Bill had a class that does X, we don't need to write a new one."
  • Sharing knowledge of good techniques and programming style.

In both these aspects, a junior reviewer tends to benefit more than a senior one.

2

Junior programmers should absolutely be performing code reviews for their senior colleagues!

However, they shouldn't be the only reviewer. Pair them up with a more experienced developer per code review.

There are a myriad of benefits:

  • The author will be forced to explain more of their code. Talking through your code is one of the best ways to find issues with it, or better ways of doing it.

  • The author will find weaknesses in their code. The junior dev is more likely to be confused by some of the more advanced chunks. Frequently, these are "too tricky" for their own good, and could benefit from simplification.

  • The junior dev will learn better coding practices. Code reviews are an opportunity to teach by example.

  • The junior dev will be a more effective code reviewer. Code reviewing is hard. The more experienced everyone is with code reviews, the faster and more effective code reviews become.

  • The junior dev will have deeper knowledge of the code base. Be selfish! By pulling the junior devs on early, you will be able to hand it off to them sooner.

  • The junior dev will feel more involved. The junior dev will start to see "senior" code (and their colleagues) as less foreign and intimidating. This is a tremendous and often overlooked benefit of code reviews.

  • The junior dev is a fresh set of eyes. They're not as indoctrinated as someone who has been working on the code base for a long period of time. The junior dev is more likely to point out different ways of accomplishing things as they ask questions. Don't shrug off their wilder comments without at least some consideration!

  • The senior devs are held accountable. I've frequently seen situations where senior devs tend to gloss over each others' code (trust, laziness, etc). An extra set of eyes helps discourage it.

The downside to consider is that all parties involved will spend a fair amount of time performing code reviews. Thus, it can be a bit of a difficult sell to management. The benefits completely outweigh the slower pace, though.

0

A code review is made for reviewing code, not for learning. If I were a junior programmer, I would be intimidated to review senior's code.

On the other hand, reading senior's code is a great way of learning, provided that the Senior is available for answering all questions.

Two alternatives could be:

  • let Juniors attend code review meetings and let every attendant be open to some teaching/learning discussions
  • practice pair programming
4
  • 8
    Code reviews can be learning experiences. That said, I fully agree, that's not their primary purpose. Ideally all team members should be involved, but I see your point, it will take a while for a (truly) junior developer to be confident enough to point out flaws (assuming she can identify them first, which is also something I wouldn't honestly expect from a junior reviewing a senior's code).
    – yannis
    Feb 12, 2013 at 10:09
  • The OP did explicitly say that the junior programmer has good skills. Less experience does not always mean lower-quality code reviews.
    – Cascabel
    Feb 12, 2013 at 17:36
  • @Jefromi: The OP did explicitly say that he/she want to set the purpose of code review to learning. I just say that this is not what they are meant for.
    – mouviciel
    Feb 12, 2013 at 19:35
  • Hm, I think we understand the OP differently - the post does say emphasis on learning, but it also says "involved as code reviewers", implying that the junior programmer is not the only person.
    – Cascabel
    Feb 12, 2013 at 22:48

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