1

I have been tasked with helping our HR department create some new interview questions for candidates applying for development positions. As part of the process, I would like to assess their ability to both understand code without the help of a compiler and spot potential future problems.

As such, I have devised a basic console application. The source code I will provide to the candidates and ask them the following questions:

  1. Would this code compile without any errors? If not, why not?
  2. Can you spot any potential issues in the code which might cause bugs in future?
  3. What improvements would you make to the code?

I've got my own answers to the questions, but I would love to get some feedback to see if there's anything that I may have missed.

using System;
using System.Collections.Generic;

namespace DeveloperTest
{
    class Program
    {
        static void Main(string[] args)
        {
            var users = new List<User>();

            users.Add(new User("John Smith", "42", DateTime.Today));

            string name = "Jane Smith";
            int age = 37;
            int yearJoined = 17;
            int monthJoined = 01;
            int dayJoined = 15;
            users.Add(new User()
            {
                Name = name,
                Age = age,
                DateJoined = new DateTime(day: dayJoined, month: monthJoined, year: yearJoined)
            });

            users[0].PrintUserInfo();

            RemoveUsersUnderAge(users, 40);

            Console.ReadKey();
        }

        private void RemoveUsersUnderAge(List<User> users, int age)
        {
            foreach (var user in users)
            {
                if (user.Age < age)
                {
                    users.Remove(user);
                }
            }
        }
    }

    class User
    {
        private string _name;
        private int _age;
        private DateTime _dateJoined;

        public User()
        {

        }

        public User(string name, string age, DateTime dateJoined)
        {
            if (name == null) Name = "Unknown";
            if (age == null) Age = 0;
            if (dateJoined == null) DateJoined = DateTime.Today;

            Name = name;
            Age = Convert.ToInt32(age);
        }

        public string Name
        {
            get { return _name; }
            set { _name = value; }
        }

        public int Age
        {
            get { return _age; }
            set { _age = value; }
        }

        public DateTime DateJoined
        {
            get { return _dateJoined; }
            set { _dateJoined = value; }
        }

        private void PrintUserInfo()
        {
            Console.WriteLine($"Name: {this.Name}.");
            Console.WriteLine($"Age: {this.Age} years old.");
            Console.WriteLine($"Joined: {this.DateJoined}.");
        }
    }
}

closed as too broad by gnat, Erik Eidt, user22815, Bart van Ingen Schenau, amon Jan 28 '17 at 22:10

Please edit the question to limit it to a specific problem with enough detail to identify an adequate answer. Avoid asking multiple distinct questions at once. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 3
    This is a weird question (I'll let others say if it's appropriate to this forum), what exactly are you trying to learn from asking this? As a candidate this could easily be a red flag: these people expect me to review fugly code, is that what I'm going to be spending my time here doing? – walpen Jan 25 '17 at 3:04
  • 2
    Do you really believe that HR will be able to understand the answers given by a candidate? E.g. if your expectation was to use int.TryParse for age in the ctor of User, but I suggest to change the type of the parameter to int, then: would HR know what to do? – Bernhard Hiller Jan 25 '17 at 9:02
  • 1
    I would get rid of the first question. Let the compiler tell me if the code doesn't compile. I'm not going to waste my time scanning every single character just to make sure I didn't forget a semicolon somewhere. With syntax highlighting, it makes even less sense to try to manually find syntax errors. – CHendrix Jan 25 '17 at 12:26
  • 2
    So, you're going to give people some code that doesn't work and has no particular purpose or design and ask them what could be wrong and improved? – Erik Eidt Jan 25 '17 at 16:05
  • 1
    Do you really want to develop & support your own tool for this when there's countless web-based, online tools for developer testing already? Your time would probably be better spent reviewing a few of them & just writing problem sets for them. – Sean McSomething Jan 25 '17 at 17:36
7

You're on the right track, but your test could use some improvement. Don't test people to understand code, test them to understand algorithms. The majority of the analysis you're asking for can be performed by tools, instead have candidates do analysis that still today require humans to perform.

The majority of the issues with your code sample will be revealed in a quarter second after hitting build and fixed nearly as quickly. Protection modifiers, static/non-static members, and typos, and similar issues are not worth spending the time going through, because it won't reveal much about the candidates overall skill, because it is mostly a typo hunt.

What you want, and your code sample lacks, is a difficult algorithm to analyze and debug. Stuff like hairy date/time handling, C string splitting algorithms, small parsing algorithms, stuff like that. Key elements are looping, branching, and data manipulation. Those are where more insidious bugs can hide, stuff like off-by-one errors, lack of bounds checking, etc. Code samples that are especially good are where only edge or corner cases are a problem.

The code sample you've provided barely does anything, the majority of it is just getters and setters, minimal decision making and nearly inconsequential looping. Additionally, since your code sample doesn't really do any particular task, suggestions for how to improve it will be contrived and meaningless. Studying your sample can demonstrate to some extent someones ability to analyze code, but not to synthesize new code. That can be demonstrated by mentally walking through complicated algorithms, where a clear model of assignment and operations can be demonstrated.

Another good way to demonstrate synthesis ability is to have part of the algorithm under analysis missing. Leave out a nested function or some such, but leave enough of the main body for it to be clear what the algorithm is supposed to be doing. Then, have the candidate write the missing piece to make the overall algorithm work again, and verify it's correctness.

The types of tasks I've outlined will do a much better job of separating the major league players from the bush leagues, and doesn't instead evaluate them on the basis of their ability to do something computers already do a million times better.

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