Interview: Review another developer's code

Question

One of my colleagues recently left and I've been asked to review the code of an interviewee coming in to replace him.

I have no idea what code he'll bring along with him and moreover have never been given this sort of responsibility before, so am eager to give some useful and fair input.

Can anyone provide a succint list of things to look out for?

I understand this is a very broad question, but this does equally seem like a very appropriate forum for asking it.

Answer 1

As this is code for an interview the candidate is probably going to bring what they feel is the best representation of their work.

Before delving into the code, ask them about it. Get them to demo it if they can. This helps you get an idea of what they have built and allow you to start picturing how you would have done it.

Next, move to looking at the code. Use the code to start a conversation. If you see something you don't understand, ask them to explain it. If you see something that you would have done differently, ask why they chose to do it that way.

You're not so much reviewing the code as you are reviewing their coding ability and their thought process. Talking about their code gives you both something to talk about, something that the candidate should know fully and that should help calm their nerves a bit.

If the candidate can give reasons why they did something, or can show how their code evolved it will give you a good idea of how strong of a developer they are and if it is someone who will fit into your team.

If the candidate dodges questions, gives vague answers, or can't really answer the 'why's of the code, this is a sign that they might be a weaker candidate.

Answer 2

Generally I would look at the design decisions made in the code rather than minor code details. We all have different style and different preferences when it comes to formatting code. However the design decisions made, which patterns have been used, and perhaps more importantly why have they been used can be important aspects. If you get a chance to ask, you can make him reflect upon his decisions.

Why use an O(x^2) algorithm when you can use an O(logn) algorithm, etc. These things can be interesting to find out.

For minor code points it really depends on what code he is expected to write, and what code he has written. Remember that even though the code he is expected to take over is of one type, it doesn't mean that the code he has written of another type is comparable. Most developers can easily adapt to others style.

Answer 3

I would mainly be looking to see how much effort had been made to make the code easy to understand. I would also look for some sort of unit tests, e.g. have they thought about testing.

(You would be amazed the number of people the submit code when there is a single static method that is many 100s of line long with no comments)

I would then ask the person at the interview to explain how the code works to check it is their own work and to see if they can reason about why they decided to write it that way.

Be careful you don’t judge someone for keeping to a coding standard you don’t like – provided they can justify what they have done and show a willingness to understand that there are other coding standard that may have value.

Answer 4

Let the candidate explain the code.

Look For/Find Out:

• The context/circumstances this was written: class assignement, personal project, released to others/actually used, etc.
• What was the time frame?
• First draft or has it been refactored?
• Consistency
• Copied from a tutorial?
• Does it work?
• Why was it written this way and what they would do differently.
• What are the strengths and weaknesses of this code and their skill set?

Suggest an alteration and see how they would handle it. Depending on the programming strengths of your supervisor, you may be able to benefit the hiring decision by providing more insight about the programmer than the program.

Answer 5

Since this is not a standard peer review of code it should operate with some different questions up front. Typically a code review is to help identify problems in the code or to seek input/feedback on how to make it better. Since this is an interviewee seeking a position, the assumption must be that the code works (or worked somewhere before it was given to you), and that the goal isn't to make the code better but to find out if the person who generated the code demonstrates the ability to supply the basic needs of the position being applied for.

Things I would look for:

1. Repetition/redundancy of logic
2. Consistency in naming conventions, formatting (consistency only)
3. Scoping/usage of variables
4. Landmarks in the code to demonstrate that the individual understands the particulars of the framework being used, particularly as they may apply specifically to the position being applied for (events, multi-threading, etc.).
5. Blatant errors, misuse of functionality, or omissions.

In an interview situation, I am less concerned with the result of the code in my hand as I am with the production of the code. If there are parts I might disagree with, rather than counting them against the individual, it might be more fruitful to allow feedback in a positive manner. E.g - "I see you used method x to achieve y. Can you talk a little bit about what led you to that choice?" If there are non-obvious parts that I agree with I might choose to do the same thing to see how the thought processes for the individual work. Questions like these also help to identify if the person who gave you the code is actually the person who wrote the code. It is not beyond some individuals to copy another person's work and attempt to pass it off as their own.

If there was a code "test" during the interview and the review was based on that, then the nature of potential bugs/errors is very important. In the end, for me, the interview code review is more about finding the programmer I need more than finding bugs and pointing out mistakes.

Answer 6

I have a great suggestion as I have gone down the same road you're going down. Many of the suggestions listed as responses to your questions thus far are great, and you should use many of these ideas. I, however, after seeing his/her code (demoed, if this is possible as well) start asking OO questions.

A couple of examples:

1. How could you take this method and make it more reusable? readable? smaller?
2. If you wanted to implement a polymorphism methodology, how would you do so with this class?

Etc., etc. Mostly, you just start talking about the code, and begin rewriting some of it with him/her to see how he/she works. They're comfortable, because we're talking their project/s. You are comfortable because you are awesome!

Answer 7

When reviewing code, use a code review checklist. Furthermore, there are a couple of things to keep in mind:

• Looking for one specific error is more effective than looking for any error in general. Depending on what your goal with the review is, it may be more effective to look specifically for null-references, for example.
• A code review generally has two goals: finding errors and providing feedback to the developer. Code reviews are a great tool to improve your programming, because it lets you know what other developers think of your solution. In your case the goal is also to provide information about the developer to the company.
• Don't get distracted by typo's, whitespace, formatting, etc. It's easy to point out a typo and don't notice the bug which is about to go in production. A typo does not cost money, a bug can cost very much.

Answer 8

Others have given great answers here, but from my experience of interviewing and the experience of friends in similar situations, one thing you need to do is to check that the code is not completely plagiarised.

You should catch it from the interview anyway, but in my experience there are a shocking number of people who are of the opinion they can blag their way into a programming role without having any actual skill in the field. Some of them have a relevant degree, others don't, some who say they do probably don't. Simply checking Google for a few vaguely unusual statements in the code may save you from hiring a Paula.

Answer 9

1. Ask the candidate to showcase his best possible code solution to a problem that he faced in his experience. See if that is appealing. Drill more, see if you have something to either appreciate or learn.
2. Take some sample code from your current code-base, where you find some code smells and take those pieces or chunks of code snippets to the prospective candidate and ask the candidate to do code review and come up with his suggestions. If he has a sense of hunch/gut-feel for code-smell and then able to identify some of those with proper reasoning, then that is cool. In this case again, if you either have a learning or appreciations, then he is the candidate whom you should embrace.

It is one thing to code well and another thing to 'identify and re-factor' code smell. And to do both well, is the sweetest thing :)

Answer 10

Check for consistency with conventions (IE capital class names, camelCase instance names). other questions to ask:-

• is it well indented?
• if it is commented, are the comments relevant?
• is the intention of the code clear?
• if there are any complex/busy looking chunks, could they have been done in an easier to read way?
• can they explain clearly what these pieces of code should do?
• do they do that?