I have been part of the interviewing team for my employer for over a decade. Over this time, I've conducted hundreds of technical interviews for programmers. By far the best advice I've ever read on technical interviewing is Joel Spolsky's The Guerrilla Guide to Interviewing. This is an essay I've shared many times with new interviewers.

The more interviews that I conduct and the more times I'm interviewed myself, the more I believe that almost everyone is unskilled at interviewing, including myself. We convince ourselves that we are good at it, or that our techniques produce superior results, but this is simply to make us feel good about our choices. A confirmatory bias. Hiring results would likely be equivalent if we just filtered for slightly above average and picked at random. Or perhaps the results would be improved, because we would remove our biases, emotions, blind spots, and body language that, unlike a psychologist or psychotherapist, we lack the professional training and practise to recognize and manage.

There is evidence that even the criteria we use to filter candidates is ineffective and that we would be better off evaluating resumes based on the number of typos, the consistency of the formatting, and the clarity of explanation, rather than degree, school, or side projects. I agree that we tend to interview too much for irrelevant skills and what the candidate currently knows, versus looking for people who are good at solving problems and who continually learn and improve.

Interviews for programmer positions involve the ubiquitous programming test. The candidate is given a programming assignment that may take anywhere from a few minutes, completed as part of the interview, or a few days, submitted before or after the interview. Asking a candidate to write code in an interview is not a bad thing. In fact, it is probably a good thing. It is even part of the Joel Test. I believe, however, that too much focus is put on the end result and not enough on the process of developing the solution. I think reviewing code with the candidate is more effective in an interview than writing code and is often overlooked. It may not lead to wildly improved hiring results, but I think it can have a positive effect, and it certainly makes for a more pleasant interview experience. This essay is a collection of my thoughts on code review as part of a technical interview and is based on my experiences as both an interviewer and an interviewee.

It May Be the Most Informative Part of the Interview

When I interview someone, I usually reserve a good portion of the interview for reviewing code. This might be a code sample that the candidate submitted as part of the interview process, or one or more of a set of short refactoring problems, in a variety of programming languages, that I have collected over time.

This is almost always the most valuable part of the interview for me, where I'm able to make up my mind as to whether or not I want to work with this person. It is much less contrived than asking trivia questions about programming languages or ah-ha questions that have nothing to do with the job at hand. Looking at code together, debugging problems, refactoring, discussing design trade-offs, is something we'll spend a lot of time doing if we work together, so I try it out in the interview to see how it goes. I like reviewing a refactoring problem because the existing code provides some context, which is more like everyday work, where to fix a bug or add new functionality, you will often review and modify code from your colleagues, legacy code, third-party or open-source frameworks, or even just your own code that you wrote two years ago.

To give you an example, I have used the following C program, borrowed from Joel Spolsky's article on Back to Basics, on many occasions.

char bigString[1000]; /* I never know how much to allocate... */
bigString[0] = '\0';
strcat(bigString,"John, ");
strcat(bigString,"Paul, ");
strcat(bigString,"George, ");
strcat(bigString,"Joel ");

For just a few lines of code, there is so much to talk about, as Joel discusses in his article. It is always interesting to see the candidate synthesize the program and it often gives me a good indication for the candidate's comfort level. I usually ask the candidate to discuss the performance and scalability of this string concatenation approach, particularly if there are thousands of strings, along with what might go wrong with this program, particularly if one of the strings was from an external source, like standard-input or a file. We can talk about performance, buffer overflow, code injection, access violations, input validation, dynamic memory allocation, a more testable approach, and even variable naming. I also like how there is a continuum of things to talk about from the straightforward to the more advanced, depending on the comfort level of the candidate. The candidate can propose basic ways to improve the program by just modifying the C code. Or the candidate can suggest refactoring the program to use C++ to make the program more efficient, expressive, and robust.

While reviewing code, I do not take an adversarial approach. I sit beside the candidate, rather than across from the candidate, and really get a feel for what it is like to work with this person. How well do they communicate ideas? Can they communicate their thought process as they problem solve, in addition to the solution they arrive at? Are they creative? How do they deal with uncertainty? Are they able to take suggestions or feedback from me? I listen to their thinking, rather than for right or wrong answers. When looking at these open-ended problems, I'm more interested in the thinking behind the solutions rather than the solutions themselves. If the communication feels natural and the candidate demonstrates clear thinking and passion, I'm usually comfortable hiring the candidate.

You Owe It to the Interviewee

I was involved in an interview where I spent a day working on a programming assignment to link elements in a sparse binary tree from left to right. I submitted my assignment and never heard back. When I followed up, the recruiter apologised for not getting back to me and cordially informed me that they did not wish to proceed with the interview. But why? Looking back on my assignment, I still feel I had a good solution to the problem and I delivered unit tests that demonstrated it worked. Perhaps I misunderstood the problem? Or, what if the interviewer never even got my code sample? What if the recruiter mixed it up with one submitted by another candidate? All of this would be cleared up if they took five minutes to review my work with me. If my solution wasn't ideal, I would have appreciated feedback regarding how it could be improved. Needless to say, this made me feel I had wasted my time and left me with a poor impression of this company. As Joel Spolsky advises, "even if they are a bad candidate, you want them to like your company and go away with a positive impression."

In another interview, I was asked to implement a thread-safe map that would expire items after a specified timeout. It basically involved implementing reader-writer locking and an asynchronous background task to expire items at the timeout, around a traditional map. I really enjoyed this assignment because it was practical — it was basically a thread-safe, in-memory object-cache — and it required the consideration of many trade-offs — performance, memory footprint, lock contention and starvation — typical in distributed system design. The feedback I got was something along the lines of "your solution was great!". When I asked if they wanted to review my code and some of the thinking behind it, the answer was basically "no, we need to move on with the rest of the interview". The interviewers may have been happy with my assignment and had an opportunity to evaluate me, but "we loved your solution!" didn't help me decide if I wanted to work there. Taking some time to review my assignment and the thinking behind it would have allowed me to get a feel for how we communicated and what understanding we had for each other and our work.

If you are going to ask someone to spend an hour, or a day, or more, writing some code for you, complete with unit tests, take at least five minutes to review it with them. If you have already decided that the candidate is not the right fit for you, these five minutes will reaffirm that decision. If not, you might just find a few good candidates from the ones you are discarding. Either way, you'll leave the candidate with a much better impression and allow them to make a more informed decision as to whether this is a place that they want to work.

You Might Learn Something

From time to time, you will interview a candidate who knows more than you about a subject. In these cases, the code review may actually teach you something. For example, perhaps you develop an application in C++ and you are proficient in traditional C++, but maybe you and your colleagues have not stayed up-to-date with the evolution of C++. Bjarne Stroustrup has commented that C++11 feels like a new language. As part of your coding assignment that you use to screen candidates, a candidate submits this Resource.h header file.

#pragma once
#include <string>

class Resource {
public:
  Resource(std::string name, std::string description);
  ~Resource();

private:
  std::string _name;
  std::string _description;
};

You might be dismissive of this candidate after looking at the signature of the constructor. Everyone knows passing strings by value can be expensive! You know these parameters should be passed by const reference instead (e.g., const std::string& name). Clearly, if this candidate can't get the little things right, there is no point in proceeding with a on-site interview. You don't even bother to open the implementation file Resource.cpp where you would see the following.

#include "Resource.h"

Resource::Resource(std::string name, std::string description)
  : _name(std::move(name))
  , _description(std::move(description))
{}

Resource::~Resource() {}

This candidate is aware that copying strings can be expensive and she is using move semantics for the sink parameters. This approach is discussed in Sean Parent's C++ Seasoning talk and he elaborates on the approach in this comment.

These C++11 techniques might be new to you, but if you took the time to code review the assignment with the candidate, you'd have a pretty different impression of this candidate after she explained her approach and the reasoning behind it. This candidate might be a great hire for your team to inject some new perspectives and help your team refresh their skills.

You Might Leave the Wrong Impression

A lot of programming tests involve some lowest common denominator in terms of the language and the problem. This usually means an algorithm or data structure problem, of digestible size, in a programming language that is broadly used. The assignment will often be divorced from the problems you'll face, or even the languages or frameworks you'll use, if you get the job.

I recently had an interview where the programming assignment was to write a singly-linked list in C. All of the methods were stubbed-out, I just had to implement each method. I was given an old laptop with a lousy keyboard and no mouse. I couldn't compile or debug the program because the environment had not been setup correctly. No expectations were communicated to me, I never met the person that evaluated my code, and the only feedback I got was that I had "passed". Half way through the time allotted for the assignment they brought me lunch. I was confused as to whether I should eat lunch or finish the assignment? I decided to skip lunch since tacos and keyboards don't always work well together.

The awkward lunch experience aside, I was forced to write code that I would never write, with no opportunity to defend it. I haven't used malloc and free in years. It has been a while since I've even used new and delete after std::unique_ptr and std::shared_ptr were introduced in C++11. A new graduate may overlook this as just another academic assignment, similar to the ones recently encountered at school — another hoop to jump through. As an experienced programmer, however, this was a very negative experience and it left me questioning whether I wanted to work there. Do they really build their own linked-lists in C? Have they heard of the Standard Template Library? Is there a reason they can't use it? I want to be surrounded by colleagues that challenge me to get better. If I work here, will I be surrounded by colleagues stuck in the dark ages?

If this employer took the time to review my assignment with me, I would have had the opportunity to explain why I would no longer write code like this and they would have had the opportunity to leave me with a better impression, by explaining why they find this an effective assignment and how it is relevant, if at all, to the work they do. Even acknowledging that the assignment was outdated would have left me with a much better impression — but certainly questioning why no one in the organization has been able to change it.

Conclusion

A programming test in an interview places too much emphasis on the employer. The candidate carefully considers and completes the assignment. The employer delivers judgment. The programming assignment can be a valuable part of the interview, but if you require it, review the assignment with the candidate. It says: I value your time, your ideas, and your work.

Even if you don't require a programming test, try reviewing code as part of the interview. Code review is a shared experience that brings value to both the employer and the candidate. You will see the thinking behind the solutions and not just the solutions themselves, you will understand how you both communicate, and how you value each other's thinking. You may find it helps you make more informed decisions on which candidates to hire. But, regardless, it will make for a more pleasant experience, you'll leave the candidate with a good impression, and you might even learn something.