7 ms·

>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

>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. 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) Look at the above. Same concept. Generators are drop in replacements for actual state. State and functions are the isomorphic (ie the same thing). Building a generator is cheaper then building a list. You should read SICP about this topic of how functions and data are the same. >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 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.

hardwaresofton 3y ago
> 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.

hardwaresofton 3y ago

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