Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question

migrated from stackoverflow.com Jul 20 '11 at 11:13

This question came from our site for professional and enthusiast programmers.

1  
What technolgy we are talking about here? Or event this one is unknown too? –  Tigran Jul 20 '11 at 11:12
3  
There's no C# 3.5. There's C# 3.0 and .NET Framework 3.5. –  Town Jul 20 '11 at 11:29
1  
@Town: When you get down to it there's no C# 3.0 either. C# is C#. The primary language hasn't really changed since they created it. –  Joel Etherton Jul 20 '11 at 11:35
9  
@Joel: Not true. The language has been changed significantly: (1) LINQ (2) automatic properties (3) implicitly typed variables and so on. –  Daniel Hilgarth Jul 20 '11 at 11:36
1  
@Rick - If you do not even know what technology this person should know how can you select somebody that knows said technology. Before you waste your time and anyone on your short list time. You need to figure out what the person should know, what they should be able to learn in say 3 months, and what technolgy they will learn. –  Ramhound Jul 20 '11 at 13:22

10 Answers 10

up vote 25 down vote accepted

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.

share|improve this answer
15  
+1. The conversation is more important than the code itself. –  MarkJ Jul 20 '11 at 11:45
5  
Also, given that this is what they believe to be the best representation of their work you shouldn't feel shy about being critical. I'd be looking for consistent and sensible coding standards being applied - if they don't do that sort of thing in code the present at an interview, they'll never do it at any other time. –  Jon Hopkins Jul 20 '11 at 13:11
    
+1 Nice approach - especially for asking the "why" question. There are usually reasons someone chooses to do things in a certain way. If they can give you a clearn concise reason why, then they probably know what they are doing. –  Catchops Jul 20 '11 at 13:48
1  
Love the answer. One addition - keep an eye out for how this candidate's code differs from what similar code might be in your company. A big point will be seeing if the candidate can successfully make the transition to your group, so if you see things done differently, it isn't wrong, but the question becomes "why?" and "will change to a new way of doing things be trivial or difficult?" –  bethlakshmi Jul 20 '11 at 14:24

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.

share|improve this answer

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.

share|improve this answer

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.

share|improve this answer

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.

share|improve this answer

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!

share|improve this answer

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.
share|improve this answer
3  
Good advice when reviewing a colleague's code. Not so useful for an interview. –  MarkJ Jul 20 '11 at 11:47
    
I had to thumbs down this since it didn't seem like you actually read the question. This is just stock code review advice that doesn't answer his question of code reviewing for an interview. I agree with MarkJ that this is good advice for reading a colleague's code, though. –  Casey Patton Jul 21 '11 at 0:20

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.

share|improve this answer
  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 :)

share|improve this answer

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?
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.