Clean Code versus Really Clever Code
Last year I found this humorously unreadable chunk of C# code on a blog. Without explanation, here’s the code:
for (Dog _dog = _animal as Dog; _dog != null; _dog = null) {
//business logic goes here
}
As we’re preparing to interview candidates to back-fill a C# web developer position here at work, I’ve decided to print out this code on a sheet of paper and have it on hand for possible use in an interview. The questions based on this code are:
1. Describe what will happen when this for loop executes.
2. What would you do if you encountered code like this in the application?
If your first reaction to the code is confusion, don’t be alarmed. Most senior developers will blink and stare at this code and not get it, and have to “mentally compile and run it” even after it’s explained. If another reaction is a desire to gag, or puke, or want to do violence to whomever wrote that, you’re also in good company.
But the question remains: what is happening here? I think this is a valid code scenario with which to quiz a C# interviewee because most of us only think of the for loop construct as being useful for iterating over a collection. Yes, you can iterate over a collection with a for loop but C# has a better tool for that: the foreach loop. What makes the for loop interesting is that it’s an assign, evaluate, reassign construct.
(If you really want to know what’s happening here, the explanation is at the end of this piece.)
Admittedly, this looks like a blatant gotcha of an interview question, but that’s not the point. The expectation isn’t for someone to explain it (which they probably won’t be able to do unless they’ve already read this: http://stackoverflow.com/a/7113387) but to be knocked a bit off balance by it before having the logic flow described, allowing them to see how and why it works. Once the process of the for loop is explained and the interviewee gets it, the table is set to ask the question that this scenario is really about: What would you do if you encountered code like this in a production application? Any reasonable variation on the following is what I’m looking for: I would re-write it as follows to make it more easily readable to every programmer after me who has to work in this part of the code base:
Dog _dog = _animal as Dog;
if (_dog != null) {
//business logic goes here
}
It’s not unusual for interviewees to write code in an interview: depending on the scenario it gives insight into the person’s thought process and problem solving skills. But I think it’s also useful to get a feel for whether a candidate would be inclined to impose readability on a block or section of obtuse code while passing through.
The real question here is: do you write clean code? Are you merely going to add code to the applications here, or are you going to make it a better code base as you go along? Any programmer can write code that a machine can understand but I want to know if you’re committed to code that can read. And if you are a clean-code programmer then I want you on my team.
(If you have not taken Cory House’s course on clean code — Clean Code, Writing Code for Humans — you really should… I can’t recommend it highly enough.)
* * * * * * * * * * * * * * *
As promised, here’s the explanation to the weird looking for loop. To understand this it’s important to remember that a for loop isn’t a mere iteration construct: it’s an initial assignment followed by an evaluate, process, re-assign loop which continues until the evaluation returns false. Let’s go through it step by step:
The initial assignment: this happens only once in the life-cycle of a for loop. We define a variable of the type Dog and, using the “as” keyword, we cast _animal to Dog and assign that to the variable _dog. If the “as” keyword in C# is new to you the key thing to know is that if _animal cannot be cast to Dog then null is returned.
The evaluation statement: _dog != null. In the previous step, if _animal can be cast to Dog, then this evaluates to true (_dog won’t be null) so code in the body of the for loop will be executed.
The reassignment statement: this might be the part that tripped you out the most. Typically this statement is something along the lines of “i++” (or “i+3” if counting by 3 makes you happier) but any valid assignment work here, including “_dog = null.” This statement runs after the first execution of the code in the body of the loop and immediately before re-running the evaluation test (_dog != null).
In simple English: if _animal is able to be cast to the type Dog then run the block of code exactly once. It’s useful to realize a for loop can be used this way, but writing code like this is extremely hard to read… so hard that even after it’s explained you probably went back to step through it in your brain. Code as obtuse as this should always be re-written to something simpler to understand.
Originally published at http://www.mikece.com on August 6, 2015.
