Hacker News Reader

Comment by incrudible | Hacker News Reader

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.

View on HN

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.

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.

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

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

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.

Reply on news.ycombinator.com