3 ms·

This is sleight of hand. The original code provides a uniform interface to create a user, with a parameter to distinguish the case. This functionality has disap

This is sleight of hand. The original code provides a uniform interface to create a user, with a parameter to distinguish the case. This functionality has disappeared in the refactoring. However, a requirement for this case-distinction must still exist somewhere else in the code, likely multiple times, otherwise we would not have a need for the original function in the first place. One thing you should avoid for readable code is callbacks and interfaces. This may run counter to advice you may find in books, but programming practice should've taught you that use of these features makes it exponentially harder to figure out "what's going on here". They are tools to solve specific problems, not defaults to apply liberally.

karatinversion 5y ago
> However, a requirement for this case-distinction must still exist somewhere else in the code, likely multiple times, otherwise we would not have a need for the original function in the first place. You must work in a nice place - I often find code that does not have an ideal layout.
- incrudible 5y ago
  Of course, code that has been speculatively laid out for a use-case that never materialized is not uncommon - a good candidate for refactoring. However, code speculatively laid out for a use-case that did materialize can prevent a refactoring. One should apply the YAGNI principle - judiciously, not dogmatically. For example, in this case there are really only two user creation types, which could be modeled with just an "isAdmin" flag. However, the likelyhood that a third type (or more) will appear down the road is high, so it is reasonable to speculatively use an enum-like type here.
- gumby 5y ago
  I don’t think that’s the gp’s point. Every call site has to decide whether the type argument is admin or user. After the change every call site has to decide which of the two create functions to call. The differentiation already existed for a reason not explicit at the place where create* was defined (although in this trivial,example it’s obvious).
antihero 5y ago
This is what struck me. All the "refactor" did was move the decision of what function to call somewhere else instead of code presented to us.
jesseduffield 5y ago
I'm afraid I'm not smart enough to deliberately use sleight of hand in one of my posts, but I take your point. I'll update the post to address your criticisms and others from this thread. My main motivation for writing the post was seeing type keys used for truly no good reason and leading to code that was impossible to understand, but the post could spend more time considering when type keys are appropriate and the downsides of alternatives.

karatinversion 5y ago

> However, a requirement for this case-distinction must still exist somewhere else in the code, likely multiple times, otherwise we would not have a need for the original function in the first place. You must work in a nice place - I often find code that does not have an ideal layout.

incrudible 5y ago
Of course, code that has been speculatively laid out for a use-case that never materialized is not uncommon - a good candidate for refactoring. However, code speculatively laid out for a use-case that did materialize can prevent a refactoring. One should apply the YAGNI principle - judiciously, not dogmatically. For example, in this case there are really only two user creation types, which could be modeled with just an "isAdmin" flag. However, the likelyhood that a third type (or more) will appear down the road is high, so it is reasonable to speculatively use an enum-like type here.
gumby 5y ago
I don’t think that’s the gp’s point. Every call site has to decide whether the type argument is admin or user. After the change every call site has to decide which of the two create functions to call. The differentiation already existed for a reason not explicit at the place where create* was defined (although in this trivial,example it’s obvious).

antihero 5y ago

This is what struck me. All the "refactor" did was move the decision of what function to call somewhere else instead of code presented to us.

jesseduffield 5y ago

I'm afraid I'm not smart enough to deliberately use sleight of hand in one of my posts, but I take your point. I'll update the post to address your criticisms and others from this thread. My main motivation for writing the post was seeing type keys used for truly no good reason and leading to code that was impossible to understand, but the post could spend more time considering when type keys are appropriate and the downsides of alternatives.