This week I've been doing phone interviews for programmers. Today we had two come in for a face to face. I was asked to come up with a "test."

My test design goal was something under twenty minutes. Rather than having them create a program from scratch, I thought it would be more revealing if they fixed something that was broke. Of course, the environment holds your hand for debugging, so I needed some way to judge competence that didn't simply involve bug chasing.

My brainstorm was to write code that, while almost entirely functional, stunk to high heaven. The applicants would be asked to refactor it. It took me about an hour to come up with some really, really ugly code. I included some great sins like cut and pasting connection strings, connecting to the database as often as possible for as long as possible, never closing connections, statically allocating arrays, bad names, every bad practice I could fit in a page full of code.

My ugly code

If you understand this code, please take a moment to fix it. I really don't think it's that hard for an experienced programmer. Let me know what you think. Then, read on.

Refactoring this proved depressingly easy. Just ignore the bad code and rewrite the thing. You essentially need code that takes a database command and returns a list of strings. There are many ways to do this, with collections its simple and even with datasets and dynamic data adapters for the terminally lazy.

Put the connection object in a method call, create database commands from the know connection, use a database join to do the work instead of all those stinking loops and you're there. Here's my solution.

Possible optimizations included caching the author result set and referencing the author ids from a dictionary object. For the GUI inclined, three tables could have been dragged into dataset and .NET would have generated table adapters and the foreign keys associations automatically. The entire dataset could have been loaded into memory; not vary scalable, but quite functional.

My candidates, both with excellent interviews, one with a great resume, the other fresh out of school, failed this miserably. In half an hour, neither got off the ground. Did they need more time? Am I missing something? I've been doing this a long time, so maybe I don't appreciate the difficulties newer eyes see. I really don't know.
ext_44932: (Default)

From: [identity profile] baavgai.livejournal.com


Yep, it's ugly by design.

The methods return an array of strings. An array is generally a fundamental list mechanism. In many languages, it contains a Catch-22; you have to allocate it before you use it, but you don't know how much you're going to use until you're done.

And experienced C# programmer would never use an array for list kind of processing, favoring either an ArrayList or List construct with a friendly Add method. Such types can spit out arrays for interoperability.

With this in mind, look at the "iterating through t1 to assign all its values to t2" bit. See that t2 is created at end of process dimensioned to the actual number of values. It does make sense, after a fashion. Note that the other method doesn't do this and just returns 100 values regardless.

The for (int i2 = 0; i2 < 100; i2++) { authors[i2] = ""; } is a bonus bad. In a C# array, all initial values are NULL. These methods ultimately fed a GUI list box that choked on the NULLs.
.

Profile

baavgai: (Default)
baavgai

Links

Most Popular Tags

Powered by Dreamwidth Studios

Style Credit

Expand Cut Tags

No cut tags