Comment by jghn

2 days ago

This is harder than it sounds, although I agree in a vacuum the idea is a good one.

So much value of the code review comes from having actual knowledge of the larger context. Mundane stuff like formatting quirks and obvious bad practices should be getting hoovered up by the linters anyways. But what someone new may *not* know is that this cruft is actually important for some arcane reason. Or that it's important that this specific line be super performant and that's why stylistically it's odd.

The real failure mode I worry about here is how much of this stuff becomes second nature to people on a team. They see it as "obvious" and forgot that it's actually nuance of their specific circumstances. So then a candidate comes in and misses something "obvious", well, here's the door.

9 comments

jghn

Reply
CharlieDigital  2 days ago

You can do code review exercises without larger context.

An example from the interview: the code included a python web API and SQL schema. Some obvious points I noticed were no input validation, string concatenating for the database access (SQL injection), no input scrubbing (XSS), based on the call pattern there were some missing indices, a few bad data type choices (e.g. integer for user ID), a possible infinite loop in one case.

You might be thinking about it in the wrong way; what you want to see is that someone can spot these types of logic errors that either a human or AI copilot might produce regardless of the larger context.

The juniors will find formatting and obvious bad practices; the senior and staff will find the real gems. This format works really well for stratification.

  • williamdclt  2 days ago

    > no input validation, string concatenating for the database access (SQL injection), no input scrubbing (XSS), based on the call pattern there were some missing indices, a few bad data type choices (e.g. integer for user ID), a possible infinite loop in one case

    I'd say all this stuff is junior-level (maybe ~mid for things like user ID integers). It's just a checklist of "obvious bad practices", it doesn't require experience.

    The senior stuff is much higher-level: domain modelling, code architecture, consistency guarantees, system resilience... system design in general.

    • CharlieDigital  2 days ago

      You can do all of that in a code review; the point is that it actually allows for better stratification because you can incorporate different challenges in a reasonable time frame and without having to do take homes and get working environments (you'll end up reviewing their code anyways in a followup session).

      1 reply →

  • jghn  2 days ago

    Yes, ish.

    In a previous job we did code review interviews. And went the route you said due to the problem I said. And yes, it's a lot better. But what also happened over time was that the bar slowly raised. Because over time the "harder" pieces of that session started to seem rote to interviewers, they became viewed as table stakes.

    Mind you this is true of any interview scheme that has a problem solving component to it. I'm not saying that the code review style is extra bad here, just that it doesn't solve this problem.

    • Hasnep  1 day ago

      I think the way to avoid the interviewer's expectations being raised over time is to write down some guidelines for what a successful candidate should be able to do. Even if you don't know how high to set the bar at the beginning, once you've hired someone you'll have at least one example of a good answer.

  • solarmist  2 days ago

    In theory, you can do code reviews without larger context if the code is carefully selected. Apparently, some companies think any badly written code from their code base can just be selected though.

conjectures  2 days ago

It's not so hard. One of the interview stages I did somewhere well known used this.

Here's the neural net model your colleague sent you. They say it's meant to do ABC, but they found limitation XYZ. What is going on? What changes would you suggest and why?

Was actually a decent combined knowledge + code question.