> Yeah but many companies don't do integration testing as the infra on this is huge and writing tests is more complicated. It's certainly overkill for take home

> Yeah but many companies don't do integration testing as the infra on this is huge and writing tests is more complicated. It's certainly overkill for take home. I would say you're wrong here. Completely. This company was not expecting integration tests as part of the project at all. > > I've worked for start ups most of my career, and I'm applying to start ups. It's mostly the huge companies that have the resources to spend the effort to have full coverage on that. > > Integration tests are Not easy, btw. Infrastructure is not easy to emulate completely, how would I emulate google big query or say aws iot on my local machine? Can't, most likely integration tests involve actual infra, combined with meta code that manipulates docker containers. I think I can agree with this -- it's true that most companies don't do it, but personally just spinning the thing up and throwing it a web request usually has rails in most languages these days. I spend more time in the NodeJS ecosystem, so I have things like supertest (https://www.npmjs.com/package/supertest https://www.npmjs.com/package/supertest) so maybe I'm spoiled. > I feel this is such a minor thing. I'm sure most people would agree if you dinged that you'd be the one going overboard, not the candidate. Yeah this is pretty reasonable -- opting in to typing at all is more a plus than a negative, on balance. > You're going to build objects dynamically every execution Anyway. This saves you the extra intermediary step of not having to save it to an extra list. It seems like I wasn't clear about how it's not about the lists/generators, so here's an explicit instance: random_string_generator (https://github.com/anonanonme/takehome-sample/blob/master/utils.py#L15 https://github.com/anonanonme/takehome-sample/blob/master/ut...) is created as a callable every single time generate_test_paths. This is unnecessary, and could be pulled out to just be a static function. If your argument is that the lambda gets optimized out, or that the benchmarked difference is insignificant (it very well could be!), then I could understand that. > It's a reasonable assumption that all users assume /xx/xx equals /xx/xx/. It's also reasonable for a client to handle a 302 redirect. The specs didn't specify the exact definition on either of these things. I'd argue the difference is so significant that it warranted a note in the documentation on the semantics, and if you google things like "trailing slash" it's been a thorn in peoples' sides for a long time. Specs did specify test cases -- and none of them had a trailing slash. I think we're really in the weeds here (in any reasonable working environment, this isn't a big deal), but it's wasteful to have every request become 2. > So? logN is super fast. You have 100 objects binary or whatever indexed insert redis uses gets there in at most 4 or 5 jumps. So even if it's the hot path redis can take it. > > NLogN is slow enough that if N is large there is noticeable slow downs EVEN when it's a single call. Recall that redis is single threaded sync. NlogN can block it completely. > > Either way this is all opinion here right? Ideally the results are returned unsorted. the client is the best place to sort this, but that's not the requirement. These are good points. Thinking about it though: - logN is not faster than O(1) - NLogN is in the API, not redis -- redis experiences N while the scan is going You're right that NLogN would certainly have a chance of blocking redis completely much more than N would, but I think in both cases redis experiences O(N) behavior for the stats endpoint, not NLogN. > Of course the act of reading and writing data will cost N regardless. But your reasoning is incorrect. Look at that picture again: https://i.stack.imgur.com/osGBT.jpg https://i.stack.imgur.com/osGBT.jpg. O(NlogN) will dwarf O(N) when N is large enough. It will even dwarf IO round trip time and has the potential of blocking redis completely on processing the sort. Ahh, see the point about N/NLogN -- it's not Redis that experiences NlogN, it's the API. Redis just happily feeds all the values down, and they're aggregated at the python level. Now, as far as the python level goes, I'm arguing that NLogN for in-memory number is going to be tiny compared to doing network requests and serializing JSON. > Sure but look at the specs. It's asking for http requests... That's reasonable -- the assumption is basically that the return fits in one request. I do think not trying to download the whole world at the same time is a legitimate concern, but it's not necessarily a pressing one since it wasn't mentioned. > I mean I could build a full logger and error handler framework here... Ah see I would expect you to pull one in -- just like you know of poetry, I'd be impressed by seeing more well-built creature comfort tools like that. Knowing good off-the-shelf tooling is a plus, IMO. I agree with you that this is probably not what they expected (it's only a 4 hour take home!) but just wanted to make the point. > For takehomes and in general people prefer readable code. Only optimize if it's needed. ... Yeah that's true, too much optimization would definitely be bad -- but my point wasn't that you should optimize, it was that the approach I was talking about could be optimized (so that would be my answer to the NLogN versus N questions). I think my version is the least surprising one -- no one has to know about pipeline or worry about atomicity. Just an O(1) operation to redis, like most people would expect to see. > Not to be insulting here. But I think your instincts are pretty off here. There's a limited amount of time assigned to this project. All the basics were included with what I did in the time allotted. > I think maybe I wasn't clear -- your instinct is what's on display. What you choose to include/use/not use is valuable signal (and that's the point of the take home). > When you have good instincts you know when to apply automated testing, when to use error handling. These aren't things you should do all the time... Well, if making sure to include automated testing for APIs and error handling is junior, I look forward to staying a junior engineer for my whole life. I don't think I'd submit a take home coding test without tests. I'm absolutely not dogmatic about it (and I have the repos to prove I don't always write tests :), but I certainly am not proud to show anyone untested code -- it is enshrined in my mind as a measure of quality. If I'm putting my best foot forward, the code will have tests, especially when it's languages with weak type systems. If the prompt was "write this like you're at a startup that has no money and no time", then sure. But even then -- technical debt forces rewrites at startups all the time, and the often the effect is worse when people don't write tests. Most of the time, you move slow so you can move fast. > You're also not testing what's traditionally tested in interviews. They test intelligence, depth knowledge and ability to design and solve problems. You're testing the ability to follow written rules. I mean following these things is trivial. It doesn't take much brain power to write error handling or unit tests. Perhaps you're just testing for someone that agrees with your programming philosophy. I mean these things can be followed regardless > I think this is the difference between take homes and whiteboard algo tests. I think the point of the take home is to see what you will do, on a realistic project that you were given full control over. That's what makes it better than the algo tests IMO -- it's more realistic and you have full control. Everyone knows they should write tests... But do you actually? Because one thing is definitely harder/takes more discipline than the other. > Your mention of integration tests is telling. I mean the code for integration tests can rival the size of the code in the project itself with the a minor benefit in safety . It's not just take home assignments but many many many companies loaded with senior engineers skip over automated integration tests and keep those tasks in todo lists for the longest time. There's a huge engineering cost to building out testing infra to the maximum ideal, and many companies just don't bother. It's mostly the bigger well established companies that have the resources to do this. Yes, but this is a small API -- you literally have to write a test that hits the server once. There are libs for doing this with flask, there is documentation showing you how. It's not rocket science, and it's crucial to catching bugs down the road. > It is worse in my opinion because it's biased. What if your opinion was more correct then my opinion but I judged you based off of my world view? Would that be right? No it wouldn't. Programmers are diverse and the best organizations have diversity in opinions and expertise. > > All in all I mostly disagree but I think your right on one thing: At least one of those job candidate evaluators that are looking at my code likely had a similar attitude as you do and I should bias my code towards that more. Likely won't write integration tests though, hard no on that one. > > Also good catch on the /test/-1/. That's definitely a legit ding imo. Just about everything with humans in the loop is biased -- that's the world we live in. I don't know if they tried to run your test, but it could have failed with a 3xx and then they disqualified you right there, or bad input or whatever. But yeah unfortunately it's hard to learn anything from this other than giving opinions on what rang bells for me. Would be awesome if they gave you feedback. Overall the code is reasonable and probably works for the normal cases -- it is a mystery why they wouldn't give feedback.