Hacker News new | ask | show | jobs
by Alifatisk 583 days ago
That's a huge improvement but damn, the fixed code didn't look any better in my eyes.

Going from

  HANDLERS = [
    Text,
    List,
    ListItem,
    Code,
    # ...
  ].freeze
to

  HANDLERS_BY_NODE_NAMES = [
    Text,
    List,
    ListItem,
    Code,
    # ...
  ].each_with_object({}) do |handler, result|
    handler::NODE_NAMES.each { |node_name| result[node_name] = handler }
  end.freeze
2 comments

I'd go with this since it's not performance critical code. Not sure if it's that more readable, but I like it better:

    BY_NODE_NAMES = HANDLERS.map {|h|
      h::NODE_NAMES.map {|n| [n, h]}
    }.flatten(1).to_h
I think we can go harder on the std lib :)

  HANDLERS.flat_map { _1.node_names.index_with(_1) }.inject(&:merge)

(nb: assuming there exists a `.node_names` to expose the constant... just because I like always using method calls)
A bigger question for me would be why the handlers don’t register themselves. It should be a very small amount of meta-programmation, and would avoid having to repeat the handlers to register them.
Self-registration is usually an anti-pattern in my experience because it introduces globals. Sometimes you can't avoid it, but if you only have a few things to register it's usually better to just list them explicitly.
Self-registration has a lot of problems.

Global state, like you say - you can't merge two apps that use the same registration mechanism but use incompatible registered sets.

Lack of discoverability in maintenance - it's a kind of COME FROM but for data.

A barried to optimization: it's not clear what will break when you remove a dependency, and you have to eagerly run all initializers everywhere - if you try to be clever and lazy, you have to pick and choose, and you're back to explicit but indirect registration.

Initialization order problems: if you have code you run during init which depends on the stuff you register at init time, you're going to have to manually manage your initialization in error-prone ways. Adding or removing dependencies may change initialization order.

> Self-registration is usually an anti-pattern in my experience because it introduces globals.

You mean unlike explicit registration introducing the HANDLERS_BY_NODE_NAMES global?

Well I guess it isn't quite as simple as I implied.

The handlers don't know anything about HtmlHandler, so you are free to make OtherHtmlHandler or whatever. The dependency direction is correct, whereas with self-registration your handlers now depend on some single unique global registry. HANDLERS_BY_NODE_NAMES doesn't affect any other code that might interact with the handlers (tests is normally the big one).

barrkei gave some very good reasons to avoid self-registration.