Ask HN: I have 10 yrs of Exp. Failed 4 takehome projects. What am I doing wrong?

https://news.ycombinator.com/item?id=36688685

by formulathree • 3 years ago

25 85 3 years ago

news.ycombinator.com

I'm actually a super senior/staff engineer with roughly a decade of experience. This current interview run has been demoralizing because I literally put a lot of effort into 4 different take home assignments and all 4 came back as failures with zero feedback given. I'm at a complete loss for what I'm doing wrong. I initially went in with a lot of confidence but I'm getting totally shut down on every submission.

Here's an example of a recent one:

https://github.com/anonanonme/takehome-sample

takehome instructions: https://github.com/anonanonme/takehome-sample/blob/master/README.md

takehome readme: https://github.com/anonanonme/takehome-sample/blob/master/documentation.md

Can anyone provide some legit criticism? Or is the person evaluating this just being unreasonable?

BA4gDY-cqjsEPWn• 3 years ago

There's a million reasons why you could and will get rejected that have nothing to do with the code you've written for the take home exercise. My usual solution is to ignore the losses and carry on with the grind. You only need to win once. Good luck!

formulathree32• 3 years ago

Thanks for the tip. Yeah I know this.

Just wondering though with 4 failures in a row, I think there is a good chance something is wrong with my code so I wanted to make sure with this post.

I've been doing the grind for years, but this is the first time I had so many take home projects.

Additionally, I never gotten hired by doing a good take-home either. Which is weird.

Additional question: What's your guys take on take-homes: Anybody ever successfully get hired by doing well on a take-home?

sircastor• 3 years ago

I think you’re unfortunately in a sellers market for jobs. It was already kind of a terrible experience, but given all the recent layoffs, you’re going up against lots of competitors, and the hiring companies don’t have a lot of incentive to provide feedback.

As ridiculous as it may sound, don’t take it personally.

Woshiwuja• 3 years ago

Honestly i dont think the code was the problem, maybe hr just didnt like you

mtmail• 3 years ago

Impressive for only 4h of work and good caveats list in the documentation. I can't see the restriction on number of path segments in the code ("/api/ followed by 1 to 6 path segments") and they might have insisted on tests while not mentioning tests. Nobody can tell what "production readiness, and code structure" might mean for them.

formulathree32• 3 years ago

Impressive for only 4h of work and good caveats list in the documentation.

Thanks! They requested me to spend only 4 hours on it.

I can't see the restriction on number of path segments in the code ("/api/ followed by 1 to 6 path segments") and they might have insisted on tests while not mentioning tests.

the segment amount is restricted via a default value in utils.py:

   def generate_test_paths(test_path_amount: int,
                        segment_amount: int = 6, #here!
                        string_pool_amount: int = 3,
                        string_length: int = 3) -> List[str]:

The segment_amount gets inserted into this lambda:

   segments = (segment_generator(random.randint(1, segment_amount)) for _ in range(test_path_amount))

which generates a list of strings from a pool of pre-selected strings.

Perhaps this section isn't clear?

Nobody can tell what "production readiness, and code structure" might mean for them.

yeah I hate that.

dustingetz• 3 years ago

orgs are chaos, “person” “reason” are both assumptions. for example perhaps there were 8 interviews that week for one position and it was earmarked for an internal candidate anyway. perhaps the position went away. perhaps your submission was too good and they hired a junior instead. perhaps the recruiter ran the standard process and the hiring manager didn’t even see your submission. perhaps your submission was best but the reviewer didn’t understand it. perhaps the reviewer was 24 years old. perhaps they hired someone who had higher status companies on their resume and that person didn’t even have to do the test. perhaps the manager is on a pet functional programming kick this year. perhaps you like FP but the manager had a bad experience with that this year. perhaps they wanted a woman. perhaps you’re a woman and they wanted a man. perhaps the manager is a red bull code smash bro. i could go on all day

formulathree32• 3 years ago

Could be, but with 4 in a row? I'm inclined to think something is wrong with my code, that's why I put an example up.

People are biased, and I'm biased as well, so I hope other people can see past it and see what's wrong with my code.

hnthrowaway0315• 3 years ago

With 10 yrs of experience maybe you can call some contact and remove the first round coding exam?

formulathree32• 3 years ago

Nah, no contacts in this company, or the ones I applied to.

I'm not exactly the most successful socially saavy engineer with a bunch of industry connections. I'm more just a lone wolf programmer. I like to think I'm really good, but guess not?

chiefalchemist2• 3 years ago

Not hearing anything has little if anything to do with you. Sadly, most companies can't be bothered to follow up in any way. Consider it a signal as to how much a organization values communication and culture, etc.

I understand. It's disheartening. But it's not you, it's them.

formulathree32• 3 years ago

Well hold on. With 4 failures in a row. It COULD be me. It's not always true that it's always them. (but I get your point and it's valid.)

That's why I posted my code. Was wondering if there's something fundamentally wrong here.

mathverse• 3 years ago

It's hard to conclude this. I had the same but applied to shitty companies (startups) who did not have much money and had over the top requirements.

chiefalchemist2• 3 years ago

Four? Not at all. You don't know what else they're looking for. You don't know who else has applied. Etc.

I think you underestimate how many comms-poor companies there are, how many shite hiring managers and processes, etc.

hardwaresofton9• 3 years ago

The code overall looks reasonable and I don’t write much python but:

- requirements.txt is the standard for specifying deps, right?

- making a README explaining the project might be good (I see you have this, but maybe switch documentation.md to README.md)

- tests?

- building the flask application via a function is a bit more testable

- reading settings from ENV is preferred (12 factor apps)

- output not sanitized for /test endpoint

- generator in util could have been pulled out probably? It’s created every time

- generate test path function seems a bit longer than it should be — all those lambdas should probably just be functions since you’re going to use them a lot

- did you have to define your own Json type? Is that complete? Where’s null?

- url generate function should probably template hostname — that’s more important than host, most of the time for running in different environments

- trailing slashes matter in flask, evidently, and every request without one gets redirected (test suite would have caught this)

- on the usage of redis, I wonder if scanning + in-memory aggregation is better… zincr/zrank/zrevrange is good, but you’ll have to hold all data in memory (and receive it in one large response) and logN anyway, might as well do it simply with a set with a dynamic prefix and scan while building the output data structure as you go.

- do your API endpoints return JSON?

- error handling around points of failure like redis

— your app goes down if the connection is flaky right?

- zrevrange is deprecated now btw

Hard to tell for far they wanted you to go, but asking might have made sense… some nice-to-haves:

- healthz endpoints?

- metrics?

- tracing?

- error reporting (ex. Sentry)

formulathree32• 3 years ago

Huge thanks for the feedback!

- requirements.txt is the standard for specifying deps, right?

poetry is a new thing. I just tried it for this project it replaces requirements.txt with pyproject.toml

- making a README explaining the project might be good

I did. It's documentation.md. I didn't title it README.md because I wanted the front page on github.com to show the takehome instructions rather then my docs on it.

- tests?

This was addressed in the documentation.md. I was given 4 hours to work on this problem so I just used manual tests.

- building the flask application via a function is a bit more testable

Flask is an IO app. It's inherently not testable via unit tests because it's a server. You'd have to build integration tests around an entire server which is huge overkill for this project. Testable logic is usually pure and stateless, that's located in utils.py. It's ok this is good criticism.

You can monkey patch or make your code 10x more complicated with modules that accept mockIO for dependency injection but I'm actually against this style of programming as it over complicates the code with little benefit.Trying to test Flask by mocking everything out is basically just testing the mocks. Better to do this haskell style and segregate IO from pure functions (see utils.py). IO can't be unit tested.

Not many people understand the "testing" philosophy I'm following here so this is good feedback. I should sometimes just follow what's popular but ultimately not the "best" way in my opinion.

- - output not sanitized for /test endpoint

this I don't get? Sanitized? The http request returns typical python structs, which are automatically converted to Json by flask/quart.

- did you have to define your own Json type? Is that complete? Where’s null?

Nope don't have to, type checking is optional in python, but why not? You have to do this to get correct type annotations to everything. You're right about this missing Null/None. It all still type checks.

- generator in util could have been pulled out probably? It’s created every time

Why pull it out? A generator is cheaper then creating a list from a list comprehension every time. instead of storing every value in a list and iterating through it, I just iterate through a generator. Saves memory and runtime cost is still O(N) regardless.

- - url generate function should probably template hostname — that’s more important than host, most of the time for running in different environments

Sure but the context is a takehome project and it's running in docker-compose as specified by the directions. I mean yes, I can make that utility function more general for sure.

- trailing slashes matter in flask, evidently, and every request without one gets redirected (test suite would have caught this)

I'm aware of this, it's not a mistake. I left it in as valid. The redirect does not get counted so api/xxx/ is equivalent to api/xxx

Your way is NlogN sorting and constant time inserts. The current way is zero cost sorting and logN inserts.

LogN is blazing fast, while nlogn can get slow if N gets too large. See this for relative visualization: https://i.stack.imgur.com/osGBT.jpg. Given the picture I would say my way is better. .

- - do your API endpoints return JSON?

Yes this is the default. If you return anything that's equivalent to that JSON type I defined in utils.py, flask will automatically return serialized json.

- error handling around points of failure like redis

Flask/quart runs under the hypercorn server (see the dockerfile). Additionally If a handler throws an exception the user automatically gets a 500 error from flask it's handled exactly as you would expect an http server should handle it. The server does not crash.

— your app goes down if the connection is flaky right?

No it does not. hypercorn remains running if the python worker crashes. But of course if hypercorn itself goes down then it's done. This of course can be mitigated by systemd but that's overkill for this project.

- zrevrange is deprecated now btw

Yeah your right. My mistake. Still works though. So it's marked for deprecation, but not actually deprecated yet.

-healthz endpoints?
-metrics?
-tracing?
-error reporting (ex. Sentry)

For a four hour take-home project? These things weren't even in the spec and how these things and if these things are implemented are extremely variable per company across the industry.

You know overall your post has been enlightening but not in the way you think because we are clashing on most of these points. I can see how developers can literally disagree on everything and how code reviewers can make a ton of assumptions and have a ton of arbitrary opinions. Not to mention varying levels of experience and what experience for that matter causes them to make completely different judgement calls. Don't take this as a slight, I'm sure if I was in your position I would be reviewing your code in the same way and you'd see me in the same way as well.

I'm starting to think take home assignments are just bad. Even worse for evaluating programmers then algorithm interviews because there's so many biased variables here that programmers are just clashing on. Maybe it's a good filter for finding programmers who view the programming world in the exact same way they see it.

hardwaresofton9• 3 years ago

poetry is a new thing. I just tried it for this project it replaces requirements.txt with pyproject.toml

Ah thanks, I saw this later -- the pyproject.toml was very illustrative.

I did. It's documentation.md. I didn't title it README.md because I wanted the front page on github.com to show the takehome instructions rather then my docs on it.

Is there a reason for this? I would think this was a good chance to show off your ability to write succinct/standardized documentation w/ sections that anyone else would be glad to read/see.

This was addressed in the documentation.md. I was given 4 hours to work on this problem so I just used manual tests.

Yeah I can see this, but I think "automated tests" are quite big in peoples' minds. Feels like it would be a checkbox on the item.

Budgeting some of the time to make 100% sure your code was tested seems like it might have been a good idea.

Flask is an IO app. It's inherently not testable via unit tests because it's a server. You'd have to build integration tests around an entire server which is huge overkill for this project. Testable logic is usually pure and stateless, that's located in utils.py. It's ok this is good criticism. > You can monkey patch or make your code 10x more complicated with modules that accept mockIO for dependency injection but I'm actually against this style of programming as it over complicates the code with little benefit.Trying to test Flask by mocking everything out is basically just testing the mocks. Better to do this haskell style and segregate IO from pure functions (see utils.py). IO can't be unit tested. > Not many people understand the "testing" philosophy I'm following here so this is good feedback. I should sometimes just follow what's popular but ultimately not the "best" way in my opinion.

I disagree here -- table stakes for a good API is integration and E2E testing -- and they're the easiest to do (no need to manipulate a browser window, etc).

I agree with you on not testing the mocks, but for that I personally would bring in a library for this like testcontainers for python: https://testcontainers-python.readthedocs.io/en/latest/READM...

IMO E2E tests are the most valuable tests.

this I don't get? Sanitized? The http request returns typical python structs, which are automatically converted to Json by flask/quart.

Sorry INPUT is what I meant to write there -- what happens if you get a -1 ?

Nope don't have to, type checking is optional in python, but why not? You have to do this to get correct type annotations to everything. You're right about this missing Null/None. It all still type checks.

Right, but this is what I mean -- when I read that I wondered if it was complete, and it wasn't. That's an unnecessary ding to take.

If you're going to type it, it should type correctly, or you should use pull in the typings from somewhere else, IMO.

Why pull it out? A generator is cheaper then creating a list from a list comprehension every time. instead of storing every value in a list and iterating through it, I just iterate through a generator. Saves memory and runtime cost is still O(N) regardless.

This is going to build objects dynamically every execution right? That's what I thought could be avoided. Less about lists vs generators, more about doing things that look like they allocate so much.

Do you think there's something you could do to create less garbage during the function execution? If not then you can disregard that point!

Sure but the context is a takehome project and it's running in docker-compose as specified by the directions. I mean yes, I can make that utility function more general for sure.

It's not about making it general per say -- it's more like what is your instinct at this point.

My instinct personally (and granted I spent like... 30min to an hour just reading all the code) is that I always take my host/port from ENV, and everything else possible. It's just something you know is going to come up.

I'm aware of this, it's not a mistake. I left it in as valid. The redirect does not get counted so api/xxx/ is equivalent to api/xxx

Ah OK, but this isn't what they asked for, right? If we want to be really pedantic, your endpoint returns a 3xx for every single error.

Redirects are not followed automatically, it depends on the HTTP client, right? So one that doesn't follow redirects would fail this test. It's unlikely, but maybe this code failed automated testing for a little reason like that.

Your way is NlogN sorting and constant time inserts. The current way is zero cost sorting and logN inserts. > LogN is blazing fast, while nlogn can get slow if N gets too large. See this for relative visualization: https://i.stack.imgur.com/osGBT.jpg. Given the picture I would say my way is better. .

The measurement endpoint will be called far more (100x, 1000x, ...) than the stats endpoint -- it's the hot path.

Also, note that sorting is zero cost, but retrieving the data will still be in N time including network round trips, which will likely dwarf any time you spend sorting in local memory. Even the conversion to JSON will dwarf the time you spend sorting for large N.

You're right about the complexity -- but note that ZREVRANGE is O(log(N)+M), and in this case we KNOW that M = N, so IIRC that's O(N).

Regardless of the complexity, I personally wouldn't make that tradeoff until it was necessary -- speed of the hot path is more important in my mind, and there are other ways to manage speed of the stats endpoint.

Also, the point of memory usage being potentially large is still relevant. My instinct is that when I see something returning a possibly very large piece of data, setting up for streaming is the way to go. Doing a scan you have the possibility to use far less memory for an individual request, and you could actually stream responses back in extreme cases, so you can avoid holding the entire result set in memory at all.

If you want to get really fancy, you can get creative with how data is stored on the redis side and get the size to N.

Flask/quart runs under the hypercorn server (see the dockerfile). Additionally If a handler throws an exception the user automatically gets a 500 error from flask it's handled exactly as you would expect an http server should handle it. The server does not crash.

Ahh OK, I did see hypercorn in the Dockerfile and forgot about it, I see how that your worker would be restarted by hypercorn.

The point about error handling still stands with respect to reporting though -- you generally want to see error handling in code, rather than just leaving it up to the runtime to fail the task and carry on.

Adding this would have prompted you to think about some sort of error reporting, which is generally good practice.

For a four hour take-home project? These things weren't even in the spec and how these things and if these things are implemented are extremely variable per company across the industry.

The things you include/don't include by instinct I think is valuable signal.

Like I said, they're nice-to-haves, but error reporting is probably the big one there. In production errors should to be gathered somewhere.

You can make the argument that they'll be collected by a logging system and that's why you've left them out, but I also don't see any structured logging.

You know overall your post has been enlightening but not in the way you think because we are clashing on most of these points. I can see how developers can literally disagree on everything and how code reviewers can make a ton of assumptions and have a ton of arbitrary opinions. Not to mention varying levels of experience and what experience for that matter causes them to make completely different judgement calls. Don't take this as a slight, I'm sure if I was in your position I would be reviewing your code in the same way and you'd see me in the same way as well. > I'm starting to think take home assignments are just bad. Even worse for evaluating programmers then algorithm interviews because there's so many biased variables here that programmers are just clashing on. Maybe it's a good filter for finding programmers who view the programming world in the exact same way they see it.

Yeah, that's a pretty common take (see all the comments in this thread).

I'd disagree that it's worse than algo interviews, because algos are rarely the most important thing in a professional code base, especially one with that's about web services and not something more demanding.

All this said though, the only way to know why they passed is to ask! Unfortunate that you can't get any feedback.

xwowsersx• 3 years ago

Flask is an IO app. It's inherently not testable via unit tests because it's a server. You'd have to build integration tests around an entire server which is huge overkill for this project.

I disagree. You could argue that the type of tests outlined here https://flask.palletsprojects.com/en/2.3.x/testing/ are integration tests, in the sense that you're actually testing a full endpoint, but it doesn't require the kind of full integration testing setup that you're describing. I'm not suggesting this is a reason you "failed" the take-home — as others have mentioned, there's myriad reasons why you could have failed, none of which have anything to do with your code, but I did feel the need to correct what I see as a wrong understanding of the type of testing setup that is required here and what's possible.

twunde• 3 years ago

I would email the recruiter back asking for feedback on the project. The worst they can say is no, and at best you get some insight into their reasoning.

Code review: For a senior dev, I would expect tests even if I didn't ask for them explicitly. I'm also surprised to see that the functions are in a generic utils instead of better named files.

The reality: 1. other people are spending more time on these takehomes. 2. Being more senior means that there are higher expectations. 3. The takehomes are typically a qualitative assessment

formulathree32• 3 years ago

I would email the recruiter back asking for feedback on the project. The worst they can say is no, and at best you get some insight into their reasoning.

Good tip. They already said in the rejection letter no feedback will be given.

For a senior dev, I would expect tests even if I didn't ask for them explicitly.

Several people mentioned this. You're definitely onto something here.

The thing here is that the code is mostly IO, not amenable to unit testing. For most web applications that has a lot of logic moves from database to web app to client to route, not much can be unit tested.

Typically the best pattern to follow here is to segregate as much logic away from IO as you can and move it into pure functions. That's what utils.py is. But you'll see utils.py is so small there's not much there to be worth testing. (Dependency Injection and mocks is a more popular pattern for unit testable code but it makes things much more complicated and you end up writing a lot of mock code and the unit tests end up testing mocks more then the actual logic itself)

The fact that utils.py is so small is an indicator that this web app is mostly an IO app and you have logic on the database dependent on logic on the server and vice versa which takes a lot of time to write "integration tests" for. Integreation tests are excessive imo for a takehome project.

Believe it or not excessive adherence to unit tests is actually a quality of junior engineer. They aren't knowledgeable enough to practice nuance and to see when unit tests become completely pointless.

But this is probably just my opinion. I think it's quite likely a lot of (senior) reviewers are following your train of thought.

tdeck• 3 years ago

Have take homes become a lot more common these days? I think I've done maybe 4 in my whole career.

formulathree32• 3 years ago

I think so. It wasn't like this before. Another thing that's getting popular is asking questions "related" to the job.

It's also kind of stupid in my opinion because of the way they pick the question. Usually they'll find the most interesting leet code style problem that their company had to solve one time in a blue moon and make that their test problem.

al2o3cr• 3 years ago

There's some nitpicky bits (like how "utils.py contains two utility functions" is then followed by _three_ bullet points :P ) but one big one that jumped out was using "zrank" to get the new score.

That's not going to return the right value most of the time - for instance, in the example from the docs at https://redis.io/docs/data-types/sorted-sets/ has this:

    zrank hackers "Anita Borg" is 4

but "Anita Borg"'s score is 1949. The operation you likely wanted was "zscore".

It would be straightforward to check this, either manually or in an automated test - simulate hitting "/api/abc/" once and "/api/xyz/" three times. The result of 3 of the 4 requests will be wrong:

    for /api/abc/: will have count=0 because /api/abc/ is the only entry in the sorted set
    first one for /api/xyz/ will have count=1 because /api/abc/ sorts before /api/xyz/ and they both have a score of 1
    second one for /api/xyz/ will have count=1 because /api/xyz/ has a higher score than /api/abc/
    third one for /api/xyz/ will have count=1 because that's still true

Now, it's true that the assignment didn't say anything about what the service should return: you might have done better on this assignment by returning HTTP 204 and an empty body.

IMO when a "super senior / staff engineer" chooses to write code, they should write WORKING CODE and verify that that's true.

formulathree32• 3 years ago

IMO when a "super senior / staff engineer" chooses to write code, they should write WORKING CODE and verify that that's true.

You can spin the code up with "docker-compose up --build"

You and everyone else can then verify your statements on my project by running this:

   docker exec -it flask poetry run http localhost:5000/api/abc/
   docker exec -it flask poetry run http localhost:5000/api/xyz/
   docker exec -it flask poetry run http localhost:5000/api/xyz/
   docker exec -it flask poetry run http localhost:5000/api/xyz/

(By the way the response on these calls actually returns the current count of the specific route for easier testing. You can see whether your hypothesis is real by watching the results unfold on each request.)

Then run this:

   docker exec -it flask poetry run http localhost:5000/stats/

and the results are:

   [
    {
        "count": 3,
        "path": "xyz"
    },
    {
        "count": 1,
        "path": "abc"
    }
   ]

Which is correct. Which means You are wrong sir. You did not test my code. Literally, I just ran this in my code, you can try it if you have the time.

I mean you're not the only person to make wrong assumptions on this thread. I'm getting the sense that reviewers just take a quick look and make rash judgements and then throw it into the trash. I'm betting it's not too far off from what the reviewer did for me.

You didn't run the code, which is fine, it's not expected of you. But shockingly, I think it's quite likely the evaluator at the company didn't run my code either and that is not expected or appropriate imo.

I think it's safe to say that This thread in general and your response included is becoming an accurate representation of typical shenanigans that happen for take home projects.

(Your mistake here fine, not a big deal, you're not evaluating me just giving me advice so getting things wrong is understandable in your case)

ihumanable• 3 years ago

So you might have an answer as to why you got a poor evaluation. Multiple people have had trouble following code, and for good reason, what's going on here?

    # - pipeline insures that the zincrby and zrank call happen atomically and prevents race conditions.
    # - zincrby increments the score on the string, zrank fetches the new score
    pipeline = redis.pipeline()
    pipeline.zincrby(URL_PATH_COUNT_DATA_STRUCTURE, 1, path)
    pipeline.zrank(URL_PATH_COUNT_DATA_STRUCTURE, path, )

    data = await pipeline.execute()

    # returns the path and current count that was just made.
    return {"path": path, "count": int(data[0])}

So you make a pipeline so that the zincrby and zrank can be atomic. But the zrank seems to be completely pointless, it does not, as the comment suggests, "fetches the new score." The reason the counts are correct is because you are pulling the first reply out of the pipeline which is the result of zincrby, which is the new score.

So if I were evaluating your code I would wonder

  - Why is there a pipeline here at all?  

  - Why is it zranking?  

  - Did the comment go stale and the candidate just has poor attention to detail?

  - Is the comment the candidate telling me that they don't know how zrank works and they don't know how pipeline works?

  - Why is the comment lying to me?

  - Does this super senior engineer think "insure" and "ensure" are the same word?

The above code seems to my reading to be identical to

    score = await redis.zincrby(URL_PATH_COUNT_DATA_STRUCTURE, 1, path)

    return {"path": path, "count" int(score)}

formulathree32• 3 years ago

There's some nitpicky bits (like how "utils.py contains two utility functions" is then followed by _three_ bullet points :P )

There are three functions: https://github.com/anonanonme/takehome-sample/blob/master/ut...

I don't judge people for these types of mistakes either. It's all nits. Especially in an HN comment. But someone evaluating a job candidate, in my opinion, needs to step up his game and I'm thinking a lot of these people are treating it the same way you're treating it.

ipaddr• 3 years ago

A take home test as a first round filter usually means they are not that serious about hiring and it costs them no time, money. Meanwhile you need to invest 4 hours (they actually want you to spend 8 hours) for a position you have no idea if you are really interested in yet.

The chance you will get ghosted? Very high.

The four hours could be better spent.. you are more likely to get a response by cold emailing other companies.

Send them open source work and ask them to judge your ability/style based on that. If they are not interested they were never going to judge your submission based om your style / abilities and the four (8) hours can be better spent.

formulathree32• 3 years ago

Good tip. I think I'm never going to do a take home assignment again.

quickthrower2• 3 years ago

This is where external recruitment agents can be a good thing. You get to talk to an intermediary (a biased self-serving one for sure) and ask them about the interview process etc. They normally have more than one thing to send you to, so happy to discount X to send you to Y which might have a better process.

When using them I find you need to keep your wits about you. Some are sharks. Always confirm whether or not they are putting you forward for something being the main thing. I have been put forward for roles without knowing then got unstuck because 2 agents put me forward for the same role.

muzani• 3 years ago

I don't know, I've been ghosted by 0% of take home tests. I usually go for them because the odds of getting in are better than resume spamming, and usually I'll be grinding leetcode anyway.

There was even one company that was clearly not a culture fit; I think the HR was a little racist, based on the tone in the email. But I passed the filter test and they probably had to interview anyway.

First round filter tests are usually along the lines of Fizzbuzz though. More than 20 mins and it's a red flag.

askafriend2• 3 years ago

I've never been ghosted by a take-home test and have gotten many offers >$500k from interviews that involved take-home tests.

This is my anecdote, you can take it for what it's worth.

eulers_secret• 3 years ago

The key distinction is “take-homes as the first round”. IME OP is right, do not do a take-home as a first interview round before you even have any chance to ask them questions. They’re not serious and will likely ghost you.

Even if they are serious, and you ask questions after the take-home then find out they don’t believe in version control and testing, you still wasted your time!

voakbasda• 3 years ago

I would assert that the mistake is accepting a take home assignment in the first place. I would walk away at the mere suggestion, because they do not respect the value of your time.

If they do not value your time while you are interviewing, there is zero reason to believe they will respect it once you are hired.

formulathree32• 3 years ago

Good call. I think I'll be following this advice for the future.

shubhamjain• 3 years ago

Honestly, your code looks great. Yes, there are thousands things that can be done better, so there will always be people saying so (just like in this thread). But that's no reason to deny a candidate a chance. Mature companies realize this and aren't really looking for the perfect code. They are more interested in the thought process behind the assignment. I think you're just having a hard luck, specially with a tight tech job market.

throwawayadvsec• 3 years ago

This kind of comments are kind of useless it should be self explanatory

" # returns the path and current count that was just made.

return {"path": path, "count": int(data[0])}"

also maybe extract variables and name them better like:

"pipeline_execution_result = await pipeline.execute()

count = int(pipeline_execution_result[0])"

same here:

    return [{"path": path.decode('utf-8'), "count": count} for path, count in response]

what's in settings.py should probably be in an env file

overall: variables and methods name could be clearer, use good names instead of bad comments, extract variables, don't define methods inside of methods

it's definitely not BAD code, but I'd expect a guy with 10YoE to do better

wruza• 3 years ago

That’s pure nagging and notmystyleism, except for the env bit.

formulathree32• 3 years ago

what's in settings.py should probably be in an env file

Have you used Django? Django follows the pattern of a settings file. No environment variables.

A global env file imo is definitively worse. It forces you to write code outside of the python ecosystem to extract these variables. Additionally python code references an environment variable that's not explicitly set by the env code could hit some logic errors or unexpected state.

What I would do if I had more time is have the settings file reference an env. The main benefit is that settings can be reused across apps and env errors are localized to a single point of failure instead of being littered throughout the app as references to env vars.

This kind of comments are kind of useless it should be self explanatory

They are useless. I agree. I put the comments there in case someone disagrees. Doesn't hurt in my opinion.

also maybe extract variables and name them better like

You mean with patten matching. No that's actually syntactically less appropriate here. The return value was not a structured product type. It was not a tuple.

The return value was a list which implies variable length. Should the list change in size that would change the pattern match. This is most likely just bad typing on the library. But my handling of said type is appropriate. Also it's super minor.

variables and methods name could be clearer, use good names instead of bad comments, extract variables, don't define methods inside of methods

I'm actually with you on this one. I prefer clear names over elegant names. But this is not overall sentiment among the majority.

Overall people prefer an elegant name over a very descriptive one simply put of some universal intrinsic ocd instinct they all have... even though short elegant names could provide zero informational value.

I'm just catering to the majority here. Make myself a comment Nazi as most people don't disparage that and use elegant names as most people prefer that over longer descriptive names.

it's definitely not BAD code, but I'd expect a guy with 10YoE to do better

Have you seen code written by people with 10yoe? What you will find is overall 10 yoe doesn't converge on your personal view perfect code.

It converges on their view which likely is wildly different from your view.

But this thread has been extremely informative on that fact seeing how literally everyone's view is completely different and how everyone thinks they're own personal view of the universe is the enlightened path.

The common theme: is lack of unit tests. But to further illustrate the diversity of opinions... You didn't even touch on testing. It took a back seat to "bad comments".

mejutoco• 3 years ago

It is common to load the variables inside the django setting files from an env files. This way in different environments you can have different values, and the secrets are not committed to the repository, but managed externally.

nprateem• 3 years ago

Your code is fine. Now you've got a decent sample, in future just respond to any other takehome requests with a link to this above, since it already shows you're competent and know how to reason.

No need to waste another 4 hours per company (a ridiculously large task).

destructuredObj• 3 years ago

I am a staff engineer with 10 years of experience who has recently gone through the hiring gauntlet and dealt with something similar.

I interviewed for a company in May that gave me a takehome that was very vague and probably would have taken my entire weekend to do it. So I declined and moved on.

My rule of thumb though is to spend no more than 2-3 hours on a takehome challenge. If you think it will take any longer, don't do it. And lack of any feedback from the company pass or fail is ridiculous to me.

As for your submission it LGTM. I'd bring you in and probably ask about how you'd want to improve it if you had more time, expecting the usual best practices and scalability things. I'm not sure your Python experience, but given the context of the problem space, I'd ask why you wouldn't consider other approaches like adding the url metric logic in a decorator or some other modularization that could be reused to "performance test" future API calls. I'd ask about your submission's lack of tests (getting at unit tests here) and how you could refactor your solution to facilitate better test coverage if you had more time. Deepdive about the tradeoffs of using Python coroutines as opposed to other concurrency methods.

Unfortunately there's always going to be a desperate developer out there who will spend every waking second overengineering a solution that will set a high standard for everyone else to be compared against.

Wishing you the best of luck!

formulathree32• 3 years ago

I'd ask about your submission's lack of tests (getting at unit tests here)

Every commenter hit on this. I have my reasons. But it's so common that next time I'll be for sure doing it.

Lesson for me and everybody: for take homes write tests.

I'd ask why you wouldn't consider other approaches like adding the url metric logic in a decorator or some other modularization that could be reused to "performance test" future API calls.

I would consider it. I just didn't do it lol. You can already use that test route to run integration tests, but the logic in that route is easily placed in its own undecorated function for usage outside of flask .

Unfortunately there's always going to be a desperate developer out there who will spend every waking second overengineering a solution that will set a high standard for everyone else to be compared against.

I actually could've spared the time for more hours on this. But they specifically requested I spend no more than 4 hours on this. I think they've gotten things that obviously took much longer then 4 hours and they didn't like it. I think maybe I should've done it anyway just for adding unit tests. They can maybe tell that something took more then 4 hours if a project took 16 hours but likely not one that took maybe 6 to 8.

abraxasquatz• 3 years ago

One easy change: You could take a bit more care to make your code tidier. As an example, you're wildly mixing " and ' without any good reason (copy paste from somewhere?). The style of your line breaks also varies a lot. Just use any formatting tool and your code will look much more cohesive. Some structure in the packaging would also go a long way (folders don't cost anything).

This might seem superficial but as a super senior staff, I'd expect that you mentor your junior coworkers in how to build large yet maintainable projects. One important aspect is readability and consistent style. That's why black becomes more and more used even though some of its rules are fugly.

Second not so easy change: I find "The code was also not fully organized for testing" - this actually hints at you either not writing a lot of tests in your day to day life or being very inefficient in producing tested code. You also could have at least included one sample test.

formulathree32• 3 years ago

The wildly mixing is because sometimes I want to use quotes within quotes: ' "a string" '.

I default to this " ". But I switch around when needed. This is standard python methodology. Anyway..

This kind of thing doesn't matter. It's pure aesthetic, it's ocd stuff in my opinion. Whether you use " or ' has zero consequence to readability, maintainability, safety, and performance. Literally zero. Your code can grow to a billion lines this still doesn't matter. This is actually an ocd instinct that you should try to separate from your rational thinking. The inconsistent quotes * feel * wrong, but rationally they have zero consequence on any metric. It's just a feeling not an actuality.

Less experienced programmers often get caught up in stylistic issues but fail to realize when style is actually of zero rational consequence. Not all styles have zero consequence but some of it does.

The code was also not fully organized for testing

Only literally 2 functions are unit testable here.

One of them is so basic that it's not even worth testing.

All unit testable functions are segregated away from IO so from a fundamental standpoint it's pretty much unit testable to the full extent.

The reason why I wrote that it's not is because the random path generation function has a bit of a flaw. It uses random number generation, so it can't be unit tested. I can't predict the output in the assert because it's random.

That's it. The amount of logic amenable to testing is so trivial and little it's not worth it here. I could segregate the random number out of the path generation function deterministic and testable but I made a judgement call here. It's a take-home and segregating the code like that leads to an API for the function call that's less intuitive. The function would need random seeds as input parameters.

Ideally, yes I shoulda done that. But this is a valid shortcut. I would say though, the majority of people aren't able to even realize how random number generation effects unit testability.

The unit testing thing is valid. Everyone mentioned that even though what's testable is so little. I'll just have to cater to the current dogma of the programming world.

iamflimflam1• 3 years ago

One thing to bear in mind is that people reviewing applicants are looking for a reason to reject applicants.

It used to be different when it was hard to find people. We would be looking for excuses to accept people - e.g. the code is a bit rough, but maybe they didn’t put too much effort into the take home - let’s get them in for an interview just in case…

Now with so many applicants it will be much more a case of who can we reject so we don’t spend all our time interviewing…

Also, don’t forget, the people reviewing your code may not be the senior engineers, bend with the wind, run your code through a standard formatted so that it will match expectations. Do the unit tests even if you think it’s pointless.

Play the game.

gassis• 3 years ago

Judging someone because they mixed double and single quotes is silly. That is easily fixable with a linter that they might have decided not to include for the sake of time.

aynyc• 3 years ago

If I was evaluating your code for production standard, then no testing is automatic failure. I assume you didn't run python formatter like black. Some of the codes aren't my personal style, but I won't deduct any points for that.

Take-home exam is bullshit in my opinion. I've done total of 3 in my career, and nothing came out of it. Zero feedback other than you are not what we are looking for. I've long since declined about any take home exams. I rather leetcode than take home because leetcode can scale, while take home is just unpaid work.

formulathree32• 3 years ago

I ran the default formatter in pycharm. Oh man someone who grades on formatting is a nightmare.

I addressed the unit testing thing in other comments. First I'm not an adherent to unit testing much of the code is io based and very little logic can actually be unit tested.

But everyone, literally everyone is calling out the lack of tests. So likely I will do it next time.

potta_coffee• 3 years ago

I've aced take homes and still been turned down for various nitpicks. A lot of times I think the person reviewing the code is looking for their pet preferences to be met rather than evaluating the potential of the candidate.

formulathree32• 3 years ago

This mirrors my current feeling. Just wanted to be sure and not biased.

I will say from the perspective of the reviewer he is likely to not be aware of this. Likely he thinks his pet preferences should be standard practice as do most programmers think of their own.

phibz2• 3 years ago

The payback on take home tests is seldom worth it. There is too much liability and risk and very little incentive to the company to provide honest feedback.

You should learn to accept the fact that you will not get an honest review. If you choose to enter in to a take home test go in to it with that understanding.

The interview process is a chance for them to review you, but also for you to review them. Make sure the process is equitable. If they ask for too much you owe it to yourself to push back. Ultimately they will respect you more if you stand up for yourself. It shows you have healthy boundaries and will protect their interests when you work for them.

achempion• 3 years ago

Is there some case you can point out about liability risk of providing honest feedback?

I've heard this argument many times but can't quite get it. Based on my experience some companies do provide honest feedback, some don't. I try to ask in advance and decline those who will not give any feedback.

phibz2• 3 years ago

Sure. What if they are undecided about you. You're good but they want to consider other applicants. In that case you'll likely get silence.

But lets say they know you're not a fit. So they decline you based on your test project. What is the benefit to the hiring manager to have them or a developer sit down and tell you why you weren't a good fit? It costs them time a lot of time. That does not benefit their need, finding someone for their open role. The biggest negative to this behavior is a reputation hit for the company with that one person.

Liability: providing feedback gives you the interviewee something to argue about. It also is a communication done by acting members of the company and therefore relatively official. All it takes is someone being a bit rude, disrespectful, or even a misunderstanding to make this now official communication something somone could try and publically shame the company with. Its easier for them to say nothing or ver little at all.

With a third party recruiter there is an expectation that the recruiter will take a percentage of the placed applicants salary. And the employer expects the recruiter will bring them quality applicants. Therefore the rectuiter often has more leverage than you might and can sometimes eek out more information for you.

Im not saying I agree entirely with this behavior. Specifically i think ghosting an applicant especially later in the process is unforgivable.

But I also think its your personal responsibility to figure out where you stand technically and socially. Find resources, people who care about you and ask them for feedback.

The interview process is terrible and its only gotten worse in the 25 years ive been working. Hang in there and do your best, but try and keep your expectations reasonable; you'll live longer.

fbrncci2• 3 years ago

Looking at the requirements, they feel very understandable to me since I have worked with Flask quite a bit. Then when I look at your repo, I see poetry and quart, both things which aren't even in the requirements. If I had written these requirements, and received your assignments, I would have passed on them the moment I see something I don't know or didn't specify (like quart).

whinvik• 3 years ago

I don't know about quart but what is wrong with having poetry. It shows that you have created a reproducible way to get the required libraries.

fbrncci2• 3 years ago

There is nothing wrong with it. I mean there are probably a few dozen others things he could have used. But if my requirements did not mention quant and poetry, while I know that I could get this to work with only docker-compose, I’d fail this take home and move to another applicant who followed the requirements and carried out the process I’m familiar with. Yes sure… he sure he shows that he knows that other tools as well… cool, but what if this was a well defined sprint ticket instead and he came back being off requirements, not matching up with the process? I’m sure there must have been other applications who didn’t diverge at all… delivered me only a working docker-compose, perfect. I’d prefer moving forward with those instead.

roland35• 3 years ago

Interviewing is a process that can make anyone, no matter how great they are, question themselves. Sometimes evaluators have some weird criteria that is important to them and you'll never know why!

jjice• 3 years ago

...came back as failures with zero feedback given.

Maybe a dumb question, but did you follow up and ask for feedback? Sometimes people just don't think to provide feedback, but they would if prompted to.

Other than that, I think the code is completely fine. Stylistically I can see people having non-consequential nit-picks based on what they like but I think it wouldn't be an issue to get weeded out by.

thdespou• 3 years ago

From your documentation: ----- Self Criticism or things I would've done if I had more time. testing: -----

That first part over there blew up your whole effort. No matter how good as an engineer you are, if you don't provide any unit tests, most often it's an automatic rejection.

Also why did you put that self-criticism section anyway? You don't owe them anything nor should they owe you as well.

As a final thought when you are doing the effort to apply to work for someone else then they can reject you for pretty much anything so don't sweat it too much.

Ask HN: I have 10 yrs of Exp. Failed 4 takehome projects. What am I doing wrong?

Related Stories