> This challenge is particularly well calibrated for an interview because there is only one correct answer: “change bool incr to int opcode” (or anything isomorphic to that). The codebase and problem statement together very clearly imply that there are currently two arithmetic opcodes, and your job is to extend that to three arithmetic opcodes. [...] Cloning even one of these functions is probably suboptimal, but I might spend twenty minutes before realizing that.
I just completed it sans writing tests (took ~30min), considered and discarded that approach, instead duplicating the code-paths (leaving incr/decr untouched and instead making mult_ versions of everything). I did reuse the delta_result_type, though.
Briefly my reasoning was that this would make the fork easier to maintain against hypothetical future upstream changes and keep the logic simpler, while guaranteeing that I didn't miss to implement any particularity - compiler would complain about missing constants or functions as long as I covered entrypoints.
A bit of rule-of-three thinking: If in the future there would be even more arithmetic functions added, maybe it would be prudent to refactor and generalize parts of it? But not at this point.
Curious to hear if you agree with OP that my approach was incorrect or with me that it's equally valid (:
It's equally valid — your reasoning exceeds our expectation for the question. Essentially we're trying to test for whether you can understand at least one correct implementation.
Whether or not your approach is "better" depends on factors that aren't part of the problem statement, so it would be a mistake to assess it as incorrect.
> instead duplicating the code-paths (leaving incr/decr untouched and instead making mult_ versions of everything)
If there already are two arithmetic opcodes, shouldn't both of them already be handled by the same code, only with an operation parameter? In that case all that should be necessary is the handling of that parameter in the place where the operation is actually performed, and you only add one more value of that parameter.
Having said that, I haven't looked and memcached yet how it actually implements these operations. I'll have to do that.
That's the problem: Since only incr and decr exist, a bool is enough to say which of these two operatios should be done. But when you add a third, the bool isn't enough. So you either change the opcode to an int, like the autor, or branch the mul instruction away above like above solution.
1. If you don't do arithmetic on it and it's not a primary key then it's not a number (eg an employee number might be "123456" but it's a string not a number); and
2. It's almost never a boolean; it's an enum.
I've lost count of the number of times I've had to change a boolean to an enum (some of which I created in the first place).
My favourite hack for this is when someone decides to add a third value to a boolean with:
Optional<Boolean> foo
Nope. You're wrong. It's even more hilarious when they add a fourth value:
Yea, I would prefer an enum over an int to choose the operation, but don’t forget that this is C, where enums _are_ ints. Oh well.
Also, that it turns out there is already an enum to extend for the binary protocol, so the blog author reused that instead of making a new one just for the this one function.
I would lean this way - I'm not a software developer by trade but I can cut and paste and look at compiler output.
Changing a bool to an int is the kind of thing that I would worry would have unknown side-effects somewhere, whereas adding new code paths is unlikely to explode the existing.
Upstream would be a consideration, and potentially seeing what kind of a patch they'd accept.
> This challenge is particularly well calibrated for an interview because there is only one correct answer: “change bool incr to int opcode” (or anything isomorphic to that). The codebase and problem statement together very clearly imply that there are currently two arithmetic opcodes, and your job is to extend that to three arithmetic opcodes. [...] Cloning even one of these functions is probably suboptimal, but I might spend twenty minutes before realizing that.
I just completed it sans writing tests (took ~30min), considered and discarded that approach, instead duplicating the code-paths (leaving incr/decr untouched and instead making mult_ versions of everything). I did reuse the delta_result_type, though.
Briefly my reasoning was that this would make the fork easier to maintain against hypothetical future upstream changes and keep the logic simpler, while guaranteeing that I didn't miss to implement any particularity - compiler would complain about missing constants or functions as long as I covered entrypoints.
A bit of rule-of-three thinking: If in the future there would be even more arithmetic functions added, maybe it would be prudent to refactor and generalize parts of it? But not at this point.
Curious to hear if you agree with OP that my approach was incorrect or with me that it's equally valid (: