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