Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

My favourite thing to do for "coding" interviews is to give the candidate a piece of absolutely awful python code that a friend of mine came up with for use for interviews.

There are code smells, side effects, errors, confusing syntax, a whole slew of things in it. I give them the code, tell them I'll help with every bit of python syntax (and really emphasise that I don't mark people down _at all_ for any lack of familiarity with python / python syntax), and ask them to work through the code and do a code review.

There's some python specific quirks that I largely consider bonus points at best, but anyone with any appreciable familiarity with at least one language should be able to spot a number of things. As they call stuff out, I'll dig in (or not) as seems appropriate.

So far it seems to be working out great for me as an interviewer. Candidates seem to be way less stressed than they are with a "whiteboard coding" exercise. Good discussions are showing me their development skills, etc.



It does seem a bit unfair - ok sure, anyone familiar with C-family syntax should be able to work through it and spot errors. But anyone already familiar with Python would be able to do so a lot faster.

I think the idea is good, but to be equitable it should be given in a language the person claims to be already familiar with. Or alternatively, only give it to people in a language they are not familiar with.


If the goal was to objectively evaluate ability in a broad population, I 100% agree. However, if my dev shop primarily uses Python then selecting for people familiar with Python is pretty reasonable.

Thankfully there is a giant corpus of terrible code out there for any language. Especially that code written by the most evil terrible coders of all time: past-self :D


As was also pointed out in the TFA, while you may not be "marking people down" because of unfamiliarity with syntax, an interviewee who has more experience with the language (and it's debugging tools) used in the interview will have an inherent advantage over someone who isn't as familiar.


Great idea, that's exactly what I like to do as well.

Because many people were interested in some actual code, here is an example that we used for Java:

---

public class UserDao {

    private Provider<Session> sessionProvider;

    public UserDao() {
        this.sessionProvider = new DbSessionProvider();
    }

    public boolean saveUser(User user, ApiConfig config) {
        try {
            Session session = sessionProvider.get();
            session.save(user);
            System.out.println("User saved!");
            return true;
        } catch (DbException e) {
            EmailUtil.sendEmail(config.getTechnicalSupportEmail(), e);
            return false;
        }
    }
}

---

"Solution": We always ask the candidate "What would you change about this code?" or something similar. We expect the candidate to come up with some selection of: - Database sessions should come an injected provider. - Configuration should probably be injected as well and not passed as a method parameter. - Booleans are not canonical Java as return value. - Don't write to stdout but to a logger. - Exception handling should probably happen outside the DAO. - Don't use static methods for sending Emails without good reason. Non-static methods are easier to test.

Finding these things is as important as being able to talk about them and give background on advantages / disadvantages of doing things different ways. With good candidates one can talk easily half an hour just about this example and adjacent topics (e.g. error handling).

---

Edit: Formatting

Edit 2: added "Solution"


This code looks like normal enterprise Java ugliness to me - I object to it on principle, but apart from having to pass ApiConfig when saving a user I probably wouldn't identify any of the official problems you listed without familiarity with the rest of the codebase.


Same here... and funny enough the only thing that really looks wrong to me is they used "this" in only one of the two places they used "sessionProvider".

It's not a bug ("this" is implied, so it works just fine), but leaving it out hints to me that it was originally a third argument to "saveUser()", and either the original person writing this didn't have a solid API in mind or "saveUser()" was intended to also be static like "EmailUtil.sendEmail()" and it was switched around later on. Overall hints towards two conflicting styles in the same codebase and no good guidelines. Or maybe there are such guidelines (as hinted in the "solution"), and whoever updated this class only did it partway. This is the point where I'd poke around in the repo history to see why it's like this and whether anything should be changed further in either direction.


The "secret decoder ring" for this sort of question is that it's really "say something intelligent about this code that indicates you have real and good experience" even more so than "find the bug". So your answer would generally be off to a good start as well.

Given the Java code in question, while I understand the idea that "well, the session provider may just be hard coded" and such, I would definitely want to hear something about deficient exception handling. It's obviously an important part of the code and while I may not be able to spew out the exact correct exception handling without knowing more about the context and what exceptions may occur, it is obviously very wrong as written.


I love the exception suppression via email.

Sr.Dev : "UserSignup failed, ask tech support what happened in our code"

Genius.


This is canonical Java. Commit and deploy!


> Booleans are not canonical Java as return value

What should you return instead? Or should the method return void and raise an exception on failure?


You could return the saved User object, if `save` changes it in any way, to allow the caller to work with it functional style, ie. make it explicit that it is an updated object (or if it is immutable, although typical persistence frameworks expect mutable objects, "bean" style).

You could also go fancy and do it properly functional, so return something like an `Either<User, Error>` instead of throwing an Exception, but that's definitely not canonical Java...


Probably boolean. If you don't see the difference, welcome to Java :D


Ah, but the code already uses boolean instead of Boolean.


Whoops, so it does. In which case I have no idea what the solution is referring to.


No serious Java code uses booleans to indicate failure or success. It's on the same level as not using a language's standard variable naming scheme. It's not wrong, but something experienced Java developers working on good code will immediately notice.

In good Java code you would return either void to indicate side effects or the newly saved user. Exception handling should be done outside the function, because depending on the context you might retry etc.

The main idea is to check if people who say they are very experienced understand conventions and have experience with common Java libraries.


Ever get the answer: this code should not exist?


"Don't write to stdout but to a logger."

Not great advice given the fact that a logger paged pretty much every SDE on this planet.


That's the intention. So people see the messages.

That said, modern Linux OSs send stdout to journald by default. journald should forward to some centralized logging server.


It's an extra dependency. But I guess we will never learn. Just import log4j and pray.


I'm dying to see this code


Yes! I love these kinds of interview questions. For some reason these feel more like a "normal" pair programming (/ code review) session, and I totally forget I'm in an interview.

Still a tiny bit stressing at the beginning while I read the code for the first time and try to understand it because I worry that I might be taking longer than the interviewer expects, but if I'm thinking out loud then that helps both of us. After I get reasonable context I can then forget about the interview and just focus on the normal pair programming / code review.


I work it BI and am hoping to expand the team. Does anyone have a good universal SQL code for that?

In my interview, I was given a T-SQL code, where I did point out the mistakes, but was told my solution wasn't optimal, but that was because I've never used that flavour before (nor was it specified in the hiring docs), so I'd like to avoid doing the same to others.


Please show some examples of such code, everyone will profit from this, no reason to hide it, thanks!


I wrote some entry-level PHP interview questions years ago and the number of people who couldn't spot simple syntax errors or identify the security hazard of echo $_GET['var'] was staggering.


Would you mind sharing it?


Right... Python.. Its syntax is so ugly. So I guess you can have really awful gems there with hidden bugs :)

I myself, shoot myself in a foot once doing Python code. I declared variable called 'len' and boom! What an interesting failure mode the script had. Took me a while to figure it out.


When I write Python it's an absolute hard line requirement to run it through Pylint and Pyright. They catch sooooo many issues you'd have to be a complete idiot not to do this (many people are unfortunately).

In this case Pylint will tell you about this mistake:

https://pylint.pycqa.org/en/latest/user_guide/messages/warni...


Hmm, nice stuff.. Luicky, I avoid python, so case closed. Ruby is so much nicer for me.


Ruby is even worse in my experience, but I guess some people have strange priorities.


Hehe.. Well, most importand is, use whatever language that makes you happy using it. I ve started using Ruby around 2006 I think. I was evaluating Python too, but syntax annoyed me and portability issues as well back then. I never regreted that I choosed Ruby. Its just solid for me :)




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: