4 ms·

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

>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. In interpreted languages I believe the thing that gets saved to mem is a pointer to the code itself. So no inherit difference here in terms of allocation. For closures reference counting is increased for the referenced variables but this is not a closure. Still this is a minor thing. Scoped functions are used for structural purposes performance benefits of using or not using them are negligible. >logN is not faster than O(1) - NLogN is in the API, not redis My point is log(n) is comparable to o(1). It's that fast. NlogN is not comparable to O(n). In fact if you look at that pic NlogN is more comparable to O(n^2). You definitely don't want NlogN on the application server whether its nodejs or python. That will block the python or node thread completely if n is large. It's preferable to use SORT in redis then in python or node because redis is highly optimized for this sort of thing (punintended) as it's written in C. Even if its not in redis, in general for backend web development you want to move as much compute towards the database and off the webserver. The webserver is all about io and data transfer. The database is where compute and processing occurs. Keep sorting off web servers and leave it all in the database. You're a nodejs dev? This is more important for node given that the default programming model is single threaded. Overall it's just better to use sets with scores because logN is blazingly fast. It's so fast that for databases it's often preferential to use logN tree based indexing over hashed indexing even when hashed indexing is appropriate. Yes hashed indexing are O(1) average case insert and read but the memory savings of tree based indexing overshadows the negligible logN cost. >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. Two things here. 1st. Sorting on the web API is definitively wrong. This is especially true in nodejs where the mantra is always non blocking code. Again the overall mantra for backend web is for compute to happen via the database. 2nd. Pipeline should 100 percent be known. Atomic operations are 100 percent required across shared storage and it's expected to occur all the time. Database transactions are fundamental and not unexpected. Pipeline should be extremely common. This is not an obscure operation. I commented about it in my code in case the reviewer was someone like you who isn't familiar with how popular atomic database transactions are, but this operation is common it is not a obscure trick. Additionally pipeline has nothing to do with your or my own implementation. It's there to make atomic a state mutation and retrieval of that state. I add one to a score then I retrieve the score and I want to make sure no one changes the score in between the retrieval and the add one. This is needed regardless of what internal data structure is used in redis. >I don't know if they tried to run your test, but it could have failed with a 3xx That'd be stupid in my opinion. 302 redirect is something I expected. Maybe I should have stated that explicitly in the docs. >IMO -- it's more realistic and you have full control. Unlikely. Even as a lead it's better to cater to the majority opinion of the team. 99 percent of the time you never have full control. Developers and leads typically evolve to fit into the overall team culture while inserting a bit of their own opinions. I'm a rust and Haskell developer applying for a python job. Do I apply my entire opinion to control the team? No, I adapt to all the bad (in my opinion) and good practices of the team. I introduce my own ideas slowly and where appropriate. > it is a mystery why they wouldn't give feedback. This is easy. It's not a mystery. It's because they don't give a shit about me. They don't want to hire me so the business relationship is over. Legally they owe me zero time to explain anything and it's more efficient for the business to not waste time on feedback. >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. No there isn't. You have to spin up redis. Flask documentation doesn't talk about that. Integration testing involves code that controls docker-compose. It's a bunch of hacky scripts with external process calls that are needed to get integration tests working. It's not rocket science but not within the scope of a take-home. Additionally hacking with docker compose is not sustainable or future proof, eventually you will hit infra that can't be replicated with docker. I will probably do it next time just to cater to people who share your viewpoints. >If the prompt was "write this like you're at a startup that has no money and no time", then sure. Lol. The prompt was write it in four hours ( no time) and they aren't paying me for it ( no money). >Specs did specify test cases -- and none of them had a trailing slash. What? In the specs all examples have trailing slashes. All of them. https://github.com/anonanonme/takehome-sample/blob/master/README.md https://github.com/anonanonme/takehome-sample/blob/master/RE... Take a look again, you remembered wrong. What the specs didn't specify was what to do when there is no trailing slash. So why not a 302? The client can choose what to do here. Either follow the redirect or treat it as an error.

hardwaresofton 3y ago
> In interpreted languages I believe the thing that gets saved to mem is a pointer to the code itself. So no inherit difference here in terms of allocation. For closures reference counting is increased for the referenced variables but this is not a closure. ... This has all been tread over before -- I won't rehash it since it seems like we're going in circles. The hot path is worth optimizing first, in my mind -- giving up perf there for stats which will be called much less doesn't make sense to me. To each their own! > 1st. Sorting on the web API is definitively wrong. This is especially true in nodejs where the mantra is always non blocking code. This assertion is so wide that it cannot be true. No matter what language you're in, you can absolutely do sorting in the API layer. > 2nd. Pipeline should 100 percent be known. Atomic operations are 100 percent required across shared storage and it's expected to occur all the time. Database transactions are fundamental and not unexpected. Pipeline should be extremely common. This is not an obscure operation. I commented about it in my code in case the reviewer was someone like you who isn't familiar with how popular atomic database transactions are, but this operation is common it is not a obscure trick. You don't need to pipeline if the operation is O(1) -- it's not a question of whether transactions are good/bad, I don't know why you're assuming people don't know about them. It's the question of taking in unnecessary complexity here when there's an O(1) operation available. You're optimizing for an operation that is not on the hot path. The reason you have to do the atomic operation is because you're using an access pattern that requires it. I know why and how you used an atomic transaction -- I disagree on whether it was necessary. > That'd be stupid in my opinion. 302 redirect is something I expected. Maybe I should have stated that explicitly in the docs. > > What? In the specs all examples have trailing slashes. All of them. This is correct, I read wrong and for that I apologize, they do have all trailing slashes. I was thinking of "/api/". > Unlikely. Even as a lead it's better to cater to the majority opinion of the team. 99 percent of the time you never have full control. > > Developers and leads typically evolve to fit into the overall team culture while inserting a bit of their own opinions. > I referred to the context of the project -- you did* have full control. If you were thinking of it in a team context, you should have asked more clarifying questions and found out more of what they wanted or would look for, similar to a real team context. > No there isn't. You have to spin up redis. Flask documentation doesn't talk about that. Integration testing involves code that controls docker-compose. It's a bunch of hacky scripts with external process calls that are needed to get integration tests working. > > It's not rocket science but not within the scope of a take-home. Additionally hacking with docker compose is not sustainable or future proof, eventually you will hit infra that can't be replicated with docker. > > I will probably do it next time just to cater to people who share your viewpoints. Again, this is a matter of what people think is hard and what isn't. Integration testing does not require that you spin up docker compose -- I literally linked to a libraries that do this in a reusable (importable) way w/ docker. You said you don't do this kind of testing regularly, so it is understandable that it's not easy for you. If docker was the problem, you can open subprocesses and assume that redis is installed in the test environment (i.e. your laptop, or CI environment), or take a binary to the redis binary path from ENV. You can even remove the burden of running redis from the test suite and just make sure it's running manually or in test suite invocation/setup. There are many options, and it's just not that hard in 2023. Don't know what (if anything) turned them off, so can't say this matters -- but it would matter to me, at the very least it would be a large plus for people who found the time to implement it. > Lol. The prompt was write it in four hours ( no time) and they aren't paying me for it ( no money). This is true, you did not have much time and they weren't paying you so I guess the situations are similar.

hardwaresofton 3y ago

> In interpreted languages I believe the thing that gets saved to mem is a pointer to the code itself. So no inherit difference here in terms of allocation. For closures reference counting is increased for the referenced variables but this is not a closure. ... This has all been tread over before -- I won't rehash it since it seems like we're going in circles. The hot path is worth optimizing first, in my mind -- giving up perf there for stats which will be called much less doesn't make sense to me. To each their own! > 1st. Sorting on the web API is definitively wrong. This is especially true in nodejs where the mantra is always non blocking code. This assertion is so wide that it cannot be true. No matter what language you're in, you can absolutely do sorting in the API layer. > 2nd. Pipeline should 100 percent be known. Atomic operations are 100 percent required across shared storage and it's expected to occur all the time. Database transactions are fundamental and not unexpected. Pipeline should be extremely common. This is not an obscure operation. I commented about it in my code in case the reviewer was someone like you who isn't familiar with how popular atomic database transactions are, but this operation is common it is not a obscure trick. You don't need to pipeline if the operation is O(1) -- it's not a question of whether transactions are good/bad, I don't know why you're assuming people don't know about them. It's the question of taking in unnecessary complexity here when there's an O(1) operation available. You're optimizing for an operation that is not on the hot path. The reason you have to do the atomic operation is because you're using an access pattern that requires it. I know why and how you used an atomic transaction -- I disagree on whether it was necessary. > That'd be stupid in my opinion. 302 redirect is something I expected. Maybe I should have stated that explicitly in the docs. > > What? In the specs all examples have trailing slashes. All of them. This is correct, I read wrong and for that I apologize, they do have all trailing slashes. I was thinking of "/api/". > Unlikely. Even as a lead it's better to cater to the majority opinion of the team. 99 percent of the time you never have full control. > > Developers and leads typically evolve to fit into the overall team culture while inserting a bit of their own opinions. > I referred to the context of the project -- you did* have full control. If you were thinking of it in a team context, you should have asked more clarifying questions and found out more of what they wanted or would look for, similar to a real team context. > No there isn't. You have to spin up redis. Flask documentation doesn't talk about that. Integration testing involves code that controls docker-compose. It's a bunch of hacky scripts with external process calls that are needed to get integration tests working. > > It's not rocket science but not within the scope of a take-home. Additionally hacking with docker compose is not sustainable or future proof, eventually you will hit infra that can't be replicated with docker. > > I will probably do it next time just to cater to people who share your viewpoints. Again, this is a matter of what people think is hard and what isn't. Integration testing does not require that you spin up docker compose -- I literally linked to a libraries that do this in a reusable (importable) way w/ docker. You said you don't do this kind of testing regularly, so it is understandable that it's not easy for you. If docker was the problem, you can open subprocesses and assume that redis is installed in the test environment (i.e. your laptop, or CI environment), or take a binary to the redis binary path from ENV. You can even remove the burden of running redis from the test suite and just make sure it's running manually or in test suite invocation/setup. There are many options, and it's just not that hard in 2023. Don't know what (if anything) turned them off, so can't say this matters -- but it would matter to me, at the very least it would be a large plus for people who found the time to implement it. > Lol. The prompt was write it in four hours ( no time) and they aren't paying me for it ( no money). This is true, you did not have much time and they weren't paying you so I guess the situations are similar.