Ayende @ Rahien

Refunds available at head office

Negative hiring decisions, Part I

One of the things that I really hate is to be reminded anew how stupid some people are. Or maybe it is how stupid they think I am.  One of the things that we are doing during interviews is to ask candidates to do some fairly simple code tasks. Usually, I give them an hour or two to complete that (using VS and a laptop), and if they don’t complete everything, they can do that at home and send me the results.

This is a piece of code that one such candidate has sent. To be clear, this is something that the candidate has worked on at home and had as much time for as she wanted:

public int GetTaxs(int salary)
{
    double  net, tax;

    switch (salary)
    {
        case < 5070:
            tax = salary  * 0.1;
            net=  salary  - tax ;
            break;

        case < 8660:
        case > 5071:
            tax = (salary - 5071)*0.14;
            tax+= 5070 * 0.1;
            net = salary-tax;   
            break;
        case < 14070:
        case > 8661:
            tax=  (salary - 8661)*0.23;
            tax+= (8661 - 5071 )*0.14;
            tax+= 5070 *0.1;
            net=  salary - tax;
            break;
        case <21240:
        case >14071:
            tax=  (salary- 14071)*0.3;
            tax+= (14070 - 8661)*0.23;
            tax+= (8661 - 5071 )*0.14;
            tax+= 5070 *0.1;
            net= salary - tax;
            break;
        case <40230:
        case >21241:
            tax=  (salary- 21241)*0.33;
            tax+= (21240 - 14071)*0.3;
            tax+= (14070 - 8661)*0.23;
            tax+= (8661 - 5071 )*0.14;
            tax+= 5070 *0.1;
            net= salary - tax;
            break;
        case > 40230:
            tax= (salary - 40230)*0.45;
            tax+=  (40230- 21241)*0.33;
            tax+= (21240 - 14071)*0.3;
            tax+= (14070 - 8661)*0.23;
            tax+= (8661 - 5071 )*0.14;
            tax+= 5070 *0.1;
            net= salary - tax;
            break;
        default:
            break;
    }
}

Submitting code that doesn’t actually compiles is a great way to pretty much ensures that I won’t hire you.

Tags:

Posted By: Ayende Rahien

Published at

Originally posted at

Comments

David Neale
09/22/2011 09:41 AM by
David Neale

Ouch, that's not exactly a skills showcase. I presume this wasn't somebody involved in OSS projects? :)

Ryan Heath
09/22/2011 09:41 AM by
Ryan Heath

Apart from the invalid code (which is perhaps translated from vb.net to c#???), the candidate used hard coded values all over the place and she did not recognize there is a lot of repeated code snippets. This is also an indication of the level of the candidate.

// Ryan

Ayende Rahien
09/22/2011 09:48 AM by
Ayende Rahien

Ryan, That is like putting a plaster on a scratch while you have an artery gushing. Yes, there are a lot of problems in the code. But, it doesn't compile!

Ayende Rahien
09/22/2011 09:48 AM by
Ayende Rahien

Ryan, That is like putting a plaster on a scratch while you have an artery gushing. Yes, there are a lot of problems in the code. But, it doesn't compile!

David Neale
09/22/2011 09:50 AM by
David Neale

PS - The method results no value so it won't compile.

Bogdan Marian
09/22/2011 09:56 AM by
Bogdan Marian

I think you are not allowed to use < in a case statement ... and of course missing the return value - that's why the code is not compiling!

@Ayende Maybe the candidate did receive a better offer and didn't want to hurt your feelings by refusing yours :)

Visar Uruqi
09/22/2011 10:05 AM by
Visar Uruqi

It seems like if he/she would use specification pattern maybe ayende would consider hiring him/her... It is indeed interesting how much information you can get from a piece of code ....

Ryan O'Neill
09/22/2011 10:08 AM by
Ryan O'Neill

I'm going to stick my neck out here, I know it does not compile and repeats logic etc but it is nearly there...

The big thing for me is I can understand what it should do by looking at it and if you refactored it to get rid if the repeated code you might lose some of that.

Would I employ them, nope. Would I write code like that, maybe (but it would compile and pass tests).

Perhaps someone has a nice refactored version which they can compare it to and I'll see the error of my ways.

Rafal
09/22/2011 10:08 AM by
Rafal

What was his/her pet project?

Frank Quednau
09/22/2011 10:09 AM by
Frank Quednau

Wow...and sigh, could not resist. did the person mean something like this?

https://gist.github.com/1234461

Yann Trevin
09/22/2011 10:09 AM by
Yann Trevin

This would make a pretty good post for The Daily WTF.

Ryan Heath
09/22/2011 10:12 AM by
Ryan Heath

@Ryan O'Neill

What I would prefer is that it should be easy to alter ranges, percentages or add new ranges without having the need to alter (some or to much) code.

// Ryan

Thomas Levesque
09/22/2011 10:13 AM by
Thomas Levesque

Even if you don't consider the invalid syntax and the lack of a return value, this is a very naive approach to the problem... clearly the work of a beginner programmer.

Ayende Rahien
09/22/2011 10:36 AM by
Ayende Rahien

Ryan, Oh, that was a great start, I agree. But submitting that at a job interview (with no time limits)?

Ayende Rahien
09/22/2011 10:36 AM by
Ayende Rahien

Rafal, Dorm management system

Ayende Rahien
09/22/2011 10:37 AM by
Ayende Rahien

Frank, Wow! That is actually extremely readable, one of the most readable versions that I have ever seen. Nice work on the "ranges"

Ayende Rahien
09/22/2011 10:38 AM by
Ayende Rahien

Thomas, And that is fine, it was a beginner's job that they applied to

Dror Helper
09/22/2011 10:45 AM by
Dror Helper

I feel your pain!

I'm currently interviewing at work and I had a candidate copy the wrong solution from the internet and then try and explain why he choose it without event understanding what he had.

tawani
09/22/2011 11:53 AM by
tawani

That is a prank.

Ademar Gonzalez
09/22/2011 12:09 PM by
Ademar Gonzalez

It doesn't compile -- er, maybe the candidate did not had a computer !?

csokun
09/22/2011 12:15 PM by
csokun

She knew that she not expected any return call.

Ayende Rahien
09/22/2011 12:22 PM by
Ayende Rahien

Ademar, It was sent over email

Phil
09/22/2011 12:56 PM by
Phil

Ayende, before giving her an interview, did you evaluate her passion based on open source contributions? :)

Amit
09/22/2011 01:19 PM by
Amit

LOL.. Don't underestimate the way of expressing the solution. :P

peter
09/22/2011 01:39 PM by
peter

I interviewed about 8 people for a position requiring "advanced sql" and "advanced C#" skills. All eight had 5+ years of sql/C# (if you believe their resumes). 7 out of 8: 1) Did not know what a self-join was 2) Used only webforms, but did not know how postbacks, page lifecycle, or viewstate worked 3) had "heard about" MVC 4) Did not have any personal projects in coding 5) had beginner-or-less javascript knowledge

Nuno Pereira
09/22/2011 01:59 PM by
Nuno Pereira

FLOAT GetTaxs( FLOAT _salary ) { FLOAT _tax = 0.0f; //change to match country taxes if( _salary > 0.0f && _salary <= 600.0f ) _tax = 0.0f; else if( _salary > 600.0f && _salary <= 10000.0f ) _tax = 3.0f; else if( _salary > 10000.0f && _salary <= 50000.0f ) _tax = 5.0f; else _tax = 7.5f;

return ( _salary * _tax ) / 100.0f; }

can i keep the job?

Valera Kolupaev
09/22/2011 01:59 PM by
Valera Kolupaev

Hey, Peter!

A Question from other side of the barricades of hiring (I am a candidate, Passionate .net, 5yrs).

Were you able do distinguish 8th candidate from the rest, solely by his resume? What part of the resume would help you to do that?

Dmitry
09/22/2011 02:05 PM by
Dmitry

I had a candidate who had to do a very simple assignment in the office. The goal was to display some data from an existing database table in a pre-defined HTML/CSS template using ASP .NET Web Forms or MVC (their choice).

She chose MVC because "she loves new technology". After about 40 minutes she told me that she cannot complete the assignment because the Visual Studio on the PC was broken. What happened was she could not drag-and-drop server controls onto an MVC view.

Bernhard Hofmann
09/22/2011 02:24 PM by
Bernhard Hofmann

To be fair, Ayende doesn't say how old she is or what experience she has. She could be a student who's been taught badly - you never know. Still doesn't excuse the code that doesn't compile.

Matthew
09/22/2011 02:51 PM by
Matthew

Frank, Love that solution for the taxes -- about the only thing I'd want to change is to make:

var taxedSlice = salary - progressionSlices[progression] > 0 ? progressionSlices[progression] : salary;

into: var taxedSlice = Math.min(progressionSlices[progression], salary);

But I'd absolutely hire you off that -- so much better than the vast majority of the 'code' that comes through.

Matthew
09/22/2011 02:52 PM by
Matthew

Ayende, What's your pre-intervew process look like? I'll be honest that I'm a bit surprised that someone who writes code like this got to an in-person interview. When doing hiring for my dept, I put a lot of effort into weeding out candidates before they make it to the office. An in-person interview is expensive (4-6 hrs of me and my team's time). The half-hour phone screen or having them submit code or complete a written test beforehand saves us a lot of time and lets us focus on the worthwhile candidates.

peter
09/22/2011 03:34 PM by
peter

@Valera

My comment was based on the interviews. I only mentioned their resumes since they all had held coding jobs for 5+ years. We do parse resumes also, mainly to see if they jump around too much from job to job. We also did a phone interview first before calling them in. The person we ended up hiring made a very nice derived table solution to my little simple sql problem, and had very definite ideas about the proper way to code. I especially liked him since he agreed to disagree with me about certain coding conventions. He was actually older than the others (around 60 I would estimate, but I didn't ask).

Andy
09/22/2011 03:41 PM by
Andy

Another minor (in the scheme of things) nitpick:

case < 5070: /* ... / case < 8660: case > 5071: / ... */

So...What happens if salary == 5070 or salary == 5071

Paleta
09/22/2011 03:56 PM by
Paleta

Dont tell me if it wouldn' be nice have that sintax on C# XD

Rich
09/22/2011 04:27 PM by
Rich

@Andy -- wouldn't that be caught by "case < 8660" though?

Noel Kennedy
09/22/2011 04:44 PM by
Noel Kennedy

Here's a functional stylee version! https://gist.github.com/1235276

Ayende Rahien
09/22/2011 04:53 PM by
Ayende Rahien

An interview would require that there is something in th CV that interests me. Itcanbecode,life or just general impression. I spend about 10 - 20 min per interview with the candidate, to get a feel for them, theask them to code some stuff

Demis Bellot
09/22/2011 06:07 PM by
Demis Bellot

Grrr, again. Posting comments really needs a preview. (feel free to nuke my previous attempts).


let taxOf = function
	| x when x < 5070m -> 0.1m * x
	| x when x < 8660m -> 0.14m * x
	| x when x < 14070m -> 0.23m * x
	| x when x < 21240m -> 0.3m * x
	| x when x < 40230m -> 0.33m * x
	| x -> 0.45m * x
	
taxOf 10000m

sympathy
09/22/2011 06:29 PM by
sympathy

Am I the only one here who feels bad for her? She probably reads this blog, and her code is being mocked.

I generally respect Ayende, and even enjoy his curmudgeonly informative attitude, but does this post really serve any purpose outside of venting some frustration?

There's not a doubt in my mind that she knew the code was poor, but for some reason or other felt pressured to submit it.

I personally would consider it a breach of trust if someone I was interviewing for posted my code as an example of what not to do.

Stan
09/22/2011 07:51 PM by
Stan

at least she have a pet project. otherwise you was not call her for interview...

Josh T
09/22/2011 08:11 PM by
Josh T

Hmm... From a readability standpoint, am I the only one who doesn't like Frank's solution at all?

If I was a developer who nothing about progressive taxing and had to come along and make a change to this code, I think that this implementation would confuse me even more.

The most glaring problem to me was that the solution had 2 independent arrays with the values declared. This would immediately lead me down a path of thinking that those arrays could change independently. Not until I went through multiple iterations of the examples in my head did it click that the code assumed you knew how the feature worked and that there is a need to keep the 2 arrays "in sync" by index, otherwise none of the logic works.

To me, those are some of the most frustrating kinds of issues to deal with, where you have to spend time realizing a developer assumption/mistake before you can see what the code is trying to accomplish.

PhilB
09/22/2011 09:15 PM by
PhilB

Here's an implementation using the LINQ extension methods. It's a bit hard to read if you ask me.

    public Decimal CalculateTax(Int32 salary)
    {
        var taxData = new[]
        {
            new { Rate = 0m,   Cap = 0 },
            new { Rate = 0.1m, Cap = 5070 },
            new { Rate = 0.14m, Cap = 8660 },
            new { Rate = 0.23m, Cap = 14070 },
            new { Rate = 0.3m, Cap = 21240 },
            new { Rate = 0.33m, Cap = 40230 },
            new { Rate = 0.45m, Cap = Int32.MaxValue },
        };

        return taxData
            .Zip(taxData.Skip(1), (prev, current) => new { Rate = current.Rate, PreviousCap = prev.Cap, BracketSize = current.Cap - prev.Cap })
            .Select(x => Math.Min(Math.Max(salary - x.PreviousCap, 0m), x.BracketSize) * x.Rate)
            .Sum();

    }
Demis Bellot
09/22/2011 09:32 PM by
Demis Bellot

Like a simpleton my previous example only calculated basic tax rate. F# is still a good choice for this, here is an example that calculates cumulative tax rate. It uses currying to compose higher order functions with different tax rates:

Gist here: https://gist.github.com/1236106

//generic routine to calculate tax
let taxOf salary taxRates = 
	taxRates 
		|> Seq.mapi(fun i (rate, band) -> 
			let prevBand = (if i > 0 then taxRates |> Seq.nth (i-1) |> snd else 0m)
			match salary with 
				| x when x > band -> (band - prevBand) * rate
				| x when x < prevBand -> 0m
				| x -> (x - prevBand) * rate
			)
		|> Seq.sum

//define custom tax bands and rates
let usTaxRates = [
	( 0.1m, 5070m); 
	(0.14m, 8660m); 
	(0.23m, 14070m); 
	( 0.3m, 21240m); 
	(0.33m, 40230m); 
	(0.45m, System.Decimal.MaxValue)]


//use currying to build a higher order function to calculate US Tax Rates
let taxOfUS salary = usTaxRates |> taxOf salary
				
taxOfUS 10000m  //= 1317.80
JustinK
09/22/2011 09:33 PM by
JustinK

PhilB - Nice solution. I'd never heard of .Zip on IEnumerable before, opened my eyes.

Noel Kennedy
09/22/2011 09:44 PM by
Noel Kennedy

Here's my earlier effort using a fold (it's called aggregate in LINQ for some reason)

public static decimal GetTaxes(decimal salary) { var taxBands = new[] { new Tuple<Decimal, Decimal, Decimal>(0, 5070, 0.1m), new Tuple<Decimal, Decimal, Decimal>(5070, 8660, 0.14m), new Tuple<Decimal, Decimal, Decimal>(8660, 14070, 0.23m), new Tuple<Decimal, Decimal, Decimal>(14070, 21240, 0.3m), new Tuple<Decimal, Decimal, Decimal>(21240, 40230, 0.33m), new Tuple<Decimal, Decimal, Decimal>(40230, Decimal.MaxValue, 0.45m) };

return taxBands.Aggregate(0.0m, (taxOwed, taxband) =>
    {
        if (salary < taxband.Item1)
            return taxOwed;

        if (salary < taxband.Item2)
            return taxOwed + ((salary - taxband.Item1) * taxband.Item3);

        return taxOwed + ((taxband.Item2 - taxband.Item1) * taxband.Item3);
    });

}

Mark Brents
09/22/2011 09:55 PM by
Mark Brents

Wouldn't it be better to just have the tax rates in a table somewhere?

Bob
09/22/2011 10:14 PM by
Bob

Publicly shaming someone like this is a bit of a prick move, sure the candidate might not be ready for the position and could have a long way to go, but if I were this candidate I would rather have someone tell me I might be doing the wrong thing and not in a public forum be made out to be an idiot.

Leszczuu
09/22/2011 11:32 PM by
Leszczuu

The problem here is her version is quite readable apart from being non-working, and other versions show that problem is not so simple. And common it's not public shaming, because none of us will ever know who's code was it. If she reads this she will know what was wrong with her code and how to fix it. It won't make her better coder instantly but she will have far greater chances when applying for a job like this next time.

Drew Wells
09/23/2011 12:31 AM by
Drew Wells

What Bob said. It's not even close to the worst code I've ever seen. At least 'she' knows how to fall through on case statements.

Besides who wants to write .NET code anyways? :P

Steve
09/23/2011 01:16 AM by
Steve

Hi Ayende,

I personally think that anyone who posts blog entries like this should at least get their spelling and grammar correct before trashing the work of others.

Cheers, Steve

Phil Bolduc
09/23/2011 02:26 AM by
Phil Bolduc

For all those who hard coded the tax table inside the method, remember, this is Ayende! Sure it compiles, but do you think he'll hire you for doing that? Either an instance field/property or better yet something that returns the tax rate based on variable parameters, such as year, state/province, country.

Luke
09/23/2011 04:25 AM by
Luke

Your demonstration of professional developer behaviour through a public review of a person rather than a constructive critique of the code has been most... enlightening... and I wish you well on your continuing journey down the road of dealing with software and humans.

Ayende Rahien
09/23/2011 04:39 AM by
Ayende Rahien

Bob, I was quite careful to give no identifying details. And I would rather have the candidate tell me that they weren't able to complete the task than send me code that didn't even compile.

Ducas
09/23/2011 06:25 AM by
Ducas

I habe to agree with Bob, Luke and the others.

This post is quite harsh and in no way constructive.

Rob Kent
09/23/2011 08:05 AM by
Rob Kent

The post may or may not be harsh, but I disagree that it is not constructive: it produced some nice functional tax algorithms.

Lee
09/23/2011 09:51 AM by
Lee

I feel extremely sorry for this person if she's read this post. You may have destroyed her confidence and affected her future in this industry.

Toni-anne Collins
09/23/2011 10:07 AM by
Toni-anne Collins

As an HR Manager of a Software Development company who also uses code tests as part of our recruitment process I was interested to know if you discussed with the candidate why they failed or asked for their permission to post the submission to your blog.

Caleb
09/23/2011 10:08 AM by
Caleb

Ayende,

Did you contact the person who submitted this and let them know what the problem was before posting it here? I would hate for them to see it on your blog without even hearing back from you.

Caleb

gunteman
09/23/2011 09:28 PM by
gunteman

Dammit, the Dark Side of the Force got hold of Yedi Ayende again.

I hate these posts. Grow up.

Brian O
09/24/2011 06:25 PM by
Brian O

Here is what would sell very easily to a business analyst at an American bank :

    private decimal calcTax(decimal salary)
    {
        decimal tax = 0;
        int salaryInt = Convert.ToInt32(salary);

         // Tax each dollar one at a time in each tax bracket.
        for (int i = 1; i <= salaryInt; i++)
        {
            if (i > 40230)
                tax += 0.45m;
            else if (i > 21240)
                tax += 0.33m;
            else if (i > 14070)
                tax += 0.3m;
            else if (i > 8660)
                tax += 0.23m;
            else if (i > 5070)
                tax += 0.14m;
            else
                tax += 0.10m;
        }

        return tax;
    }

It taxes each dollar one at a time. I did a performance test using 80,000 dollars, and Visual Studio 2010 cannot detect a difference between this and the "Slices" version even down to the Tick count of the CPU.

I guess we can call this one "The Buffet Function". It taxes 800,000 bucks at $352,568.10.

Ashaq Ali
09/24/2011 09:04 PM by
Ashaq Ali

I Believe it should work and calculate the tax, i know code is ugly, but u cant write better ode u r bored and checking the random blogs, was searching for signalR Example and got hooked to this one, strange but true.

static void Main(string[] args) { float[,] slabs = new float[5, 2] { { 5070, 0.10f }, { 3590, 0.14f }, { 5410, 0.23f }, { 7170, 0.30f }, { 18990, 0.33f } };

        float income = 50000f;
        float totalTax = 0;
        for (int i = 0; i < slabs.GetLength(0); i++)
        {
            float currValue = slabs[i, 0];
            float currTaxSlab = slabs[i, 1];

            if (income >= currValue && (i != (slabs.Length - 1)))
            {
                income = income - currValue;
                totalTax += currValue * currTaxSlab;

            }
            else if (income < currValue && (i != (slabs.Length - 1)))
            {
                totalTax += income * currTaxSlab;
                income = 0;
            }

        }

        if (income > 0)
        {
            totalTax += income * 0.45f;
        }
        Console.Write("Total Tax: {0}", totalTax.ToString());
        Console.ReadKey();
    }
majid rafie
09/25/2011 07:24 AM by
majid rafie

sorry for my bad English... this code is full of syntax errors...:D incorrect use of switch-case syntax , declare method by int return type and then return a double value,net and tax is not Initialized, ...

GetTax method can simply wrote:

public decimal GetTax(decimal sal) { var taxFactor = new[] { new { fac = 0.45m, Lim = 40230m }, new { fac = 0.33m, Lim = 21240m }, new { fac = 0.3m, Lim = 14070m }, new { fac = 0.23m, Lim = 8680m }, new { fac = 0.14m, Lim = 5070m }, new { fac = 0.1m, Lim = 0m }

                        };

        decimal tax = 0;
        foreach (var item in taxFactor)
        {
            if (sal > item.Lim)
            {
                tax += (sal - item.Lim) * item.fac;
                sal = item.Lim;
            }
        }

        return tax;
    }
iwayneo
09/27/2011 10:34 AM by
iwayneo

even the method name has a spelling mistake.

Lee
09/27/2011 02:51 PM by
Lee

@Toni-anne Collins Permission to post the code? Good question, maybe Ayende should require candidates to sign some kind of agreement saying that the code submitted may or may not be published, that will definitely weed out candidates, but there might not be any candidates then? :)

Anyway, no personally identifiable information, no defamation, no harm, if she does read the blog, there have been plenty of good solutions to learn from. As for, '...destroyed her confidence and affected her future in this industry.', a bit dramatic, we're not in high school.

jernejg
09/28/2011 07:05 AM by
jernejg

@ayende How do you feel about googling the problem and copy/pasting the solution. I mean if you give a candidate a standard problem to solve on an interview, like this tax thingy or a basic algorithm problem described from Knuuth's book are you really just testing his coding skills or are you also testing his skills of finding information on the web and optionaly modifying it.

Or to put it in other words. Would you hire a programmer that would finish the test in 5 minutes and say "Here is the solution. The task that you gave me was a standard xx problem, so I googled the code and copy/paste it".?

Ayende Rahien
09/28/2011 07:48 AM by
Ayende Rahien

Jernejg, If he manages to do that ,good for him. I would add a twist to the problem, something new. For the tax problem, it would be persisting the tax rates, for an algorithm, it would be asking about the edge conditions and handling them, etc.

CLee
09/29/2011 07:34 PM by
CLee

Ask yourself this, is your blog post going to encourage people (even those that have the skills) to want to work for/with you? Your right, she doesn't have the developer skills to get the job, but then again, she and others probably wouldn't want the job now after considering your social skills.

James Culbertson
10/02/2011 01:19 PM by
James Culbertson

For those criticizing Ayende for posting this get real. If he was hard on the candidate he is doing him/her a favor by letting them know that they are either in the wrong field or need to rededicate a little bit to their craft and do better. Either way it would be no favor to the candidate to not tell them the truth. Political correctness and dancing around the issue will not be good for anyone involved and doesn't advance the field of software development.

Comments have been closed on this topic.