HACKER Q&A
📣 crthrowaway

How to review the code of someone much smarter/more experienced?


Some background: I think I'm a solid coder. I went straight from my CS degree into software development and have spent the last 18 years coding, and (hopefully!) getting better along the way. I've sometimes felt I've been the best programmer on a team, but certainly not always. If I had to take a SWAG and put a broad number on it, I'd say I'm a 60-70th percentile programmers.

I recently changed jobs and became the third member of a team. The existing members are an older team lead who seems to be an ok coder, and someone about my age and experience level, who is the office rock star. The rock star eats, sleeps and breathes Java and the related technologies. He cranks out a lot of code, and he works a lot (he seems to have little else to do during the pandemic). He considers the system we're working on "his baby". The 2-person team had already been doing code reviews before I arrived.

I'm not a Googler, but I've read and try to stay somewhat close to their "How to do a code review" guide. One item that sticks with me from their guide is "Look at every line of code that you have been assigned to review". I try to stick to that and not just scan and rubber-stamp reviews, but man is it hard when you log in in the morning and find 3 new MRs with a total of about 3000 lines to review. Especially when that feels like a common occurrence.

The reality is, as the new person on the team, I'm certainly not going to deeply understand the implication of every change. But I feel that it's my job to dive in and look for any issues I can find. The issue here is that code review is taking up an inordinate amount of my time, meaning I'm not making what I consider to be ample progress on my coding tasks.

Does anyone out there have pointers for how to do effective code reviews in situations like this?


  👤 detaro Accepted Answer ✓
A few points, you need to judge how well they apply to your situation and your relationship with the coworkers:

a) have you discussed the expectations on your code review (and the time it takes)? A clear "take the time you need" or "focus on bits that look critical" might answer that. (Certainly many places would be fine with someone new taking time to understand what's going on, but if you are worried it might be good to get clarification on that)

b) similar, what kind of focus do they expect from code review? E.g. I know places that take stylistic things very seriously, and others don't at all. Stylistic stuff is on one hand more to think about, but also an easy place to show "yes, I'm reviewing this".

c) Asking people to change their workflow is always a bit tricky, but if you think it helps maybe you could ask for more smaller MRs - that makes changes easier to think through properly IMHO.

d) are the changes missing context for you to understand? "I don't get why this is correct, can you please expand the comments/commit message" is IMHO valid feedback.

e) hopefully as you learn more about the structure you can prioritize yourself better, e.g. by recognizing that changes fit established patterns and don't need as much attention.


👤 FroshKiller
The key thing to me is that everyone on the team should be able to understand what the code is doing. If I am out and a team member can't read my code to troubleshoot a critical issue, I have let my team down. If I change projects and the next programmer can't easily pick up my work, I have let my team down.

Approach it from that perspective. If a "rock star" is coding above someone's head and making sweeping changes, that is a liability. The moment you feel out of your depth, that is the heart of the code review. A line-by-line reading from that point is a waste of your time.

If the 22-year-old junior on my team said to me, "I tried to review your code, but I don't understand this section with the generic functions," that to me would be the value--not whether I followed our guidelines or made some minor mistake. That's an opportunity for me to improve the maintainability of my code and help my teammate improve their knowledge.

You should be able to carry out your code review with this teammate in that way with confidence in my opinion. If you feel like you can't, that seems dysfunctional to me.


👤 yokaze
Consider your inexperience an asset in this context. The changes and the code should be (to some degree) self-explanatory.

The star may crank out code like no other, but chances are they don't want to maintain it.

So the review from your side can help to relinquish that responsibility.

The point is though, there needs to be common understanding in that matter. (Also that it'll will take your time). It might make sense to talk about that with your colleagues and find an agreement.