Hacker News Reader

Comment by raganwald | Hacker News Reader

The article's point about writing code that does what it says it does is fine. But as an interview question, I have trouble imagining that candidates won't figure out that this is a game of "guess the answer I'm looking for" and say that they would rewrite this code.

A better question would be, there's tremendous deadline pressure, the company is in imminent danger of losing a giant deal if we don't have working code for some demo, and you have three features to implement by Monday afternoon. Do you write a new feature immediately, open a ticket for refactoring this loop and then write a new feature, or rehearse your explanation to the big boss that over the lifetime of the software, rewriting the code before adding a new feature was more important?

http://raganwald.posterous.com/javas-comb-over

Just kidding, but trying to make the point that "what do you think of this code" is a little obvious as an interview question.

View on HN

Most do say exactly that, then we have a chat about what they might do instead. Many immediately say, "um, is there a Clear or RemoveAll method?!" It's all over within a few minutes, we move on. The bigger picture is always going to trump the details, until the day the details have piled up and can no longer be ignored. It's the great big technical-debt elephant in the room.

I expected that would be the case, just wanted to make it obvious. Meta point: Context matters for code as well as interview questions :-)

I think saying "yes, there's a Clear, but what would you do if there weren't", or "yes, there's a Clear - how would you implement it if you made the library?". Interview questions where the right answer is "I know an API or assume something about it correctly" are stupid, but questions where that is the right answer with a more interesting follow-up are not.

The answer isn't "I know an API or assume something about it correctly". The answer is "Gawd that's fucking awful, I can't believe there's not a clear() method. I'd go look, before reinventing the wheel. And if lib was internal I'd suggest adding the clear() method instead of adding horrible loops all over the code <mentions DRY>. And if 3rd party lib I'd question the quality and if we really should be using that lib".

In otherwords showing confidence, humility, communications, and that they have a clue about quality.

Yes, that would be a great answer, and ideally they get to the part you mentioned about adding it to an internal library, to which the next question is, Ok, you're adding it, how do you write it? My point is merely that it's still worthwhile to see an implementation, rather than moving on when you hear the word "Clear".

Any implementation coded up on the fly, from memory in 30 min or less is crap. I don't want to hire people willing to write crap, so I don task them to.

The correct answer is "This code looks ugly, and bad".

Thank you for making this as simple as it should be.

Agreed. "What do you think of this code?" is not a hard question.

Surely the right answer would be something to the effect of "If this code survived in the source base until deployment, all is lost anyway."

The point of code review is to catch monsters like this when they are written. If that doesn't happen, what's the point?

That's a very purist view of the world.

This code works and, as he says, was produced by a coder under time pressure. You show me a system and pretty much I'll show you a system that has poor (but working) code in it produced by a competent developer under pressure.

And code review is a useful process but it's no guarantee that issues will be caught any more than system testing or user acceptance testing.

And yet the world keeps on spinning and all is not lost.

I agree, you'd hope this was caught, but if someone gave that answer in an interview even aside from the tone, I'd wonder how much real world experience they had.

If the candidate picks up on the fact that the code is ugly and should be rewritten, and then is able to explain why, then I think the question is worth it. It weeds out the people who honestly don't see anything wrong with that kind of code.

I am not a programmer, so I did not know about the clear() method. I thought it was a rather elegant solution! I guess it's all about perspective.

The problem with this code is that it is quite obfuscated. The "i" variable is essentially always 0, because it is being incremented and decremented over and over. Here is code that does the same thing, but without the useless variable:

   while( this.MyControl.TabPages.Count >0)
   {
      this.MyControl.TabPages.Remove ( this.MyControl.TabPages[0] );
   }

Reply on news.ycombinator.com