elmarschraml.com

Occasionally updated personal site and blog

Hiring developers? How to do code reviews in technical interviews

A significant part of our technical interviews is having the candidate do a code review on a piece of example code. I’ve written before about why we do that. Today I’ll share a few tips on how to make it work well.

The code

The code you give the candidate to review needs to be long enough to include enough things to talk about, but short enough to understand quickly, and keep in your head. The code we are currently using is a little under two pages when printed out. We could probably strip some of the boilerplate and go to one page – but anything less than a page is probably not enough.

It should NOT be a complicated problem domain – the review will be about the code, not the specific problem the code solves. But ideally it belongs to an industry your company typically works in. If you get a candidate that has worked in your industry before, you also get to see how familiar they are with topics and problems typical for that industry. If they have not, I of course expect no knowledge about that domain – but a senior candidate should realize where they need to ask for domain knowledge.

Do not use code that implements a complicated algorithm – you don’t want candidates to spend their time trying to understand it. But for most candidates it is easier if what the code does actually makes sense. At first I used something where the business logic was intentionally nonsense in order to make clear that it was not about the logic, but a lot of candidates actually got stuck trying to make sense of it. So now I’ve streamlined the logic to the point where it is still dependent on unknown requirements, but seems consistent.

It works best to pick a class that you wrote for a real project, strip it down to a suitable length, and then intentionally mangle it to include lots of questionable code that should get flagged in the review.

The issues

The most important thing is not to prepare a quiz (“Find the 10 mistakes hidden in the code”), but use the code as an invitation to start a discussion.

Include lots of different things that might draw the candidate’s attention – from commenting to formatting to syntax to logging to exception handling to null safety to naming to nested method calls to method length to duplicate code to performance. This makes it easier for candidates to find something to start the discussion with. And you also get to see not only what they notice, but what KIND of things they notice – a candidate that talks about ways to optimize performance by inlining a method call or using a different list implementation is probably a completely different kind of programmer than someone who focuses on method length and variable names.

Include issues that are suitable for different levels of experience. For people straight out of university, I include a few language constructs that are fairly basic and taught in any university course, but rarely used in real life. For experts in the particular language, I include some obscure syntax constructs that most people won’t recognize – but I’m delighted whenever somebody does. And for experienced programmers coming from other languages, there are a few deeper issues that are language-agnostic but should be red flags for candidates that are experienced enough that they have probably been bitten by that before.

Most of the review works regardless of what language a candidate is used to. It makes no sense for a Lisp programmer to review a piece of code written in Java – but for somebody coming from a background similar enough that the code is recognizable, we just ignore anything language-specific and focus on design and general coding principles.

The most fun topics are those where the code is not necessarily right or wrong, but subject to either personal taste or project requirements. With a senior candidate, I frequently even get new ideas or learn something new. 

If you are working in a specific industry, you might also include some issues where somebody with experience in that industry could be expected to recognize some typical problems or questions. For example, when your review code deals with testing, it could have some areas that are likely to cause false positives. Or if you are dealing with payments, an experienced candidate should probably bring up issues like fraud or interrupted requests.

Doing the review on paper, with everybody having their own copy, actually works great, and is much easier than dealing with projectors or screen sharing. Just be sure to print in color, with good syntax highlighting, and to include line numbers so everybody can follow along. If your co-interviewers are not yet familiar with this interview style, or the particular piece of code being used, it’s also helpful to maintain a separate interviewers’ version of the code that includes comments about the things that are intended to be found in the interview. 

Of course, before letting the code to be reviewed loose on candidates, try it out a few times with co-workers at different levels of seniority, both to get a feel for the amount and level of issues you can expect candidates to find, and also to make sure that it is possible to quickly grasp the code and find some issues with it. 

How to perform the code review

Before the review, I ask candidates which language they are strongest in. If the code to be reviewed is in Java, but their best language is python, I’ll skip the parts about syntax or language-specific details. If they claim to be an expert in Java with 10 years of experience, we might end of going deep into the details of e.g. various implementations of List. 

It is important to point out the basic premises of the review, so they do not get lost:

This is not a checklist test, but a pretty open-ended basis for a conversation about code quality.

Feel free to cover any aspect of “Code Quality” you can think of  (for junior candidates, I also mention a few examples like readability, structure, error-safetz etc to help them get started – whereas senior candidates should be able to define code quailty themselves)

It is not about syntax, the logic or algorithm, but about quality

The code is definitely sub-optimal, and you are expected to be critical (especially important to mention when interviewing junior developers, who might be shy about criticizing other peoples’ code)

The best way to introduce the basic premise is a story like this:

“Imagine that you have been working with us for a while, and are now an experienced engineer. This piece of code was written by an intern – he was very motivated, but is also inexperienced, so there are probably many things he missed. You are all that stands between this code and production. 

Your job is to do a quality check, for any aspect of quality you can think of. There is no checklist of “10 things to find” – there are many different things one could talk about, and often also aspects that are open to discussion, or a matter of taste, or depend on context and requirements. This is not about syntax – you can assume the code compiles without errors. You do not know the requirements – this is not about the business logic, you can assume that I ran the code in the most obvious good case, and it worked. 

Feel free to think out loud, and start by just mentioning things that seem less than ideal, or strange, to you, and we’ll use that to start the conversation”

The last point is important – many, especially junior candidates, get hung up on trying to read and understand the whole code before saying anything. It helps then to subtly prompt them with “Which line are you at right now, and what do you think about it?” or even point to something specific like “Anything strange about that switch statement on line 17?”. After a minute or two it then becomes clear what is expected, and most candidates will walk you through the code by themselves. And the most interesting topics often start with an off-hand comment like “hmm, that seems weird…”

Try to turn the review into a conversation, by not just noting issues they found, but talking about how they would improve it, what alternative solutions exist, or even talking about which requirements would lead one to choose that kind of solution. 

15 minutes, including the introduction, is usually enough to get a good signal on the candidate’s aptitude. If you have to stop because you’re running out of time, that’s usually a good sign, that you’re both having fun and interesting discussions. 

Things to check for in the code review

What kind of things do they notice? Are most of their comments along the lines of “this method is way too long, and javadoc is missing”, or more like  “we could optimize performance by saving some method calls here”?

What size of things do they notice? Small details like a missing import statements, or major bugs in the control flow?

How proactive are they? Do they just comment “this code is really bad”, or provide a well-reasoned argument why,  and suggest improvements?

Do they find solutions? I do not want to ask trivia questions about e.g. the syntax of regular expressions, but if a complicated piece of code could be replaced with a trivial reges, it’s good to see that someone has regexes in their mental toolbox, and suggests to use them. 

How deep is their knowledge? You can alway dig deeper on any concept they find. For example, a piece of code where you should really, really have used a Hashmap could be just noted, or the better the candidate is the more likely you can use it as a jumping-off point to go deeper into e.g. data structures, or performance characteristics of sorting algorithms, or the benefits of multi-threading, or the possibility of a bloom filter as a heuristic alternative, or whatever else the candidates has deep knowledge in. 

Do they have good taste? Do they care to make code that is clean, well-structured and easy to understand, or “if it runs, it’s good”?

Are they current? While I do not want to make syntax trivia too much of a focus, it’s always good to include either a bit of syntax that is obviously outdated to see if they suggest replacing it with something newer, or include something brand new to see if they recognize it. 

The range of results between candidates is huge – I have had everything from “can barely even read the code, does not know anything to say about it” to “races through everything I ever thought someone could mention, including the most obscure tidbits I don’t really expect anybody to find, in 10 minutes”. And the best candidates have pointed out flaws I did not even include intentionally, or suggested possible improvements I never thought about before. But even weak candidates can, with some prompting, find at least a few things to talk about, so nobody has to walk out of the interview feeling like a total failure. 

Ideally, the code review is both fun and a learning opportunity for both sides. For me, a job interview should feel as little as possible like an exam, and instead be a conversation among experts to judge each other’s level. Pretty much what I imagine university graduation exams would be like in a perfect world – not “answer these 100 questions”, but “let’s have a conversation, and see if you can hold your own among professionals.” Doing a code review is the best way I have found to get there. 

And if you are doing code reviews in job interviews, I’d love to hear your tips at askanengineeringmanager@gmail.com

Comments are closed.

%d bloggers like this: