I threw the docs in the README for the submission, but for HN readers they want to see the problem first. The submission didn't even include the specs.
>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.
Yeah you're completely right here. I'm gonna go with it next time.
>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).
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.
>Sorry INPUT is what I meant to write there -- what happens if you get a -1 ?
You get a 500. Which is not bad. But you're right a 400 is better here.
>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.
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.
>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.
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.
numbers = [i*2 for i in range(500)] #allocated list in memory: 500*sizeof(int) (big)
for i in numbers:
print(i + 1)
numbers = (i*2 for i in range(500)) #allocated generator in memory: sizeof(*generator) (small)
for i in numbers:
print(i + 1)
>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.
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.
>The measurement endpoint will be called far more (100x, 1000x, ...) than the stats endpoint -- it's the hot path.
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.
>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.
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. 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.
>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.
Sure but look at the specs. It's asking for http requests. It's not a streaming protocol here. I mean at this point if you were evaluating my code for a job you'd be going to far already as your going off the rails completely.
>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.
I mean I could build a full logger and error handler framework here. I'd put custom handling in a decorator and have the error handling invisible in the http route handlers. Flask already does this, but it just returns 500 on all exceptions. That is the most elegant way imo. But again this is a bit outside of the scope of a takehome, given the framework handles a lot it and the extra effort required.
>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.
For takehomes and in general people prefer readable code. Only optimize if it's needed. I mean I can write the whole thing in assembly code for ultimate performance. I can def see some nitpicker dinging me for optimized but unreadable code.
>The things you include/don't include by instinct I think is valuable signal.
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.
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. Your attitude is something I see in many younger engineers. This attitude of rule following and best practices is just adhered to without thinking it all through properly. For the scope of the project. most of what you wrote here isn't needed.
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
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.
>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.
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.