Should Junior Programmers be involved as code reviewers in the projects of Senior Programmers?

Question

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?

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 '13 at 10:04
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 '13 at 10:07
But sometimes this title is determined by HR politics and games :) — Michal Franc, Feb 12 '13 at 10:09
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 '13 at 10:30
@ThomasOwens, By "junior programmers", I mean the people with less experience in industry. — Md Mahbubur Rahman, Feb 12 '13 at 10:37
I don't know english very well. As I know senior programmer means who has good programming skill. doesn't it? — Sungguk Lim, Feb 23 '13 at 20:42
@sunglim not necessarily: A senior programmer is someone who has been programming for a long time (in general, or on the project). A junior programmer will have less experience - but they still might have good skill. — Nevir, Aug 04 '13 at 21:04
We have had accountant participate in code reviews, who's only "programming" experience was in Excel formulas and macros. Why? Because she had the exceptional ability to parse boolean logic and identify unhanded cases. Don't let something as trivial as a *job title* keep skilled eyes from reviewing production code. — recursion.ninja, Aug 10 '13 at 21:54
I have only 1 year experience in Android Development. But my senior who has 10+years of experience, "for convenience sake at that time" took a Boolean value true, converted it to string "true" and did value.equals("true") for an if condition. I cant reject that pull request since I am a Junior, but I raised it to him personally, and he said, its a convenience factor and will not affect my part of the program. — Jude Osbert K, Aug 14 '19 at 05:12

Stephen C · Answer 1 · 2013-02-12T23:04:43.027

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.

edited Feb 12 '13 at 23:04

answered Feb 12 '13 at 10:44

Stephen C

25,180
6
64
87

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 '13 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 '13 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. – Michael Shopsin Feb 12 '13 at 19:26

score 61 · Accepted Answer · answered Feb 12 '13 at 12:29

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...

answered Feb 12 '13 at 12:29

Thomas Owens

79,623
18
192
283

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. – Md Mahbubur Rahman Feb 12 '13 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. http://programmer.97things.oreilly.com/wiki/index.php/Code_Reviews – pdr Feb 12 '13 at 12:37
2

@pdr I consider violations of coding standards to be defects (it's even defined that way in PSP and TSP). So considering that your #2 and #3 are rolled up together into my concept of a defect, I think that bumps it ahead of knowledge sharing. That's not to say that knowledge isn't shared - at the level of detail of a code review, it's hard to get an understanding of the system as a whole (which I think is necessary to understand the implementation details). From the perspective of the junior engineer, until they are familiar with the system, a code review is a poor method of knowledge sharing. – Thomas Owens Feb 12 '13 at 12:40
1

I don't. Not at all. For one thing, I said "establishing" a coding standard, meaning through team discussion. Not through one individual defining a set of standards and everyone else following. So I question your rolling them together. But even if you do, I don't put that ahead of knowledge sharing. – pdr Feb 12 '13 at 12:43
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 '13 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 '13 at 12:51
2

@pdr There should be organizational recommendations across project. In critical systems, using a decision analysis and resolution process to evaluate options and choose the best one is key. Nothing is determined in a code review, but at the appropriate time in planning, after requirements are specified, or based on system architecture. A code review only looks at written code to ensure it conforms to the mandated style, the system design, and is defect free. If you're making decisions about what unit test framework or what library to use after code is written, you're doing things out of order. – Thomas Owens Feb 12 '13 at 13:03
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 '13 at 13:10
I say that unit-test framework is a bad example while sitting in a team where different projects use different frameworks; at least one project uses both. Code-reviews would have raised the discussion long before it became the problem it is now. – pdr Feb 12 '13 at 13:14
I agree with @pdr in that code reviews facilitating the shared ownership of the code is a high priority. Only having those who understand the code doing code reviews can create an echo-chamber effect and unintentionally create sub-teams. I don't believe the differences in priorities (finding bugs v. shared ownership) are quantifiable or empirically relevant. – David Kaczynski Feb 12 '13 at 16:48
The question isn't about *required* participants - it's about whether a talented but less experienced person should be *also* be involved. – Cascabel Feb 12 '13 at 17:38
1

@Jefromi And I answered that. A junior engineer may have the knowledge necessary to aid in the completion of the purpose of the code review - finding errors in the implementation of a design. Even if they do not have that knowledge, there may still be some value in including them, assuming you consider the risks associated with inviting them. – Thomas Owens Feb 12 '13 at 17:50

Simon Martin · Answer 3 · 2013-02-19T16:30:21.940

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.

edited Feb 19 '13 at 16:30

answered Feb 12 '13 at 12:52

Simon Martin

1,006
8
18

6

Now that's a good opening sentence. – pdr Feb 12 '13 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. – kevin cline Feb 12 '13 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. – Bryan Anderson Aug 03 '13 at 19:00

score 25 · Answer 4 · answered Feb 12 '13 at 18:37

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.

score 13 · Answer 5 · edited Feb 17 '13 at 10:45

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.

score 7 · Answer 6 · answered Feb 12 '13 at 10:49

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.

score 5 · Answer 7 · answered Feb 12 '13 at 16:37

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:

Ensure code is correct
Get the writer to think about how others will see their code
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.

score 2 · Answer 8 · answered Feb 12 '13 at 19:31

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.

score 2 · Answer 9 · answered Aug 04 '13 at 20:51

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.

score 0 · Answer 10 · answered Feb 12 '13 at 10:06

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

answered Feb 12 '13 at 10:06

mouviciel

15,473
1
37
64

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 '13 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 '13 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 '13 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 '13 at 22:48

Should Junior Programmers be involved as code reviewers in the projects of Senior Programmers?

10 Answers10

Linked