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

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?

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]:

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

Perhaps this section isn't clear?

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

yeah I hate that.

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.

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?

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

>- 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

- 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.

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....

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.

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.

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/

Then run this:

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

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

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)

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.

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.

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.

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

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".

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.

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.

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.

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.

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.