laughinghan/mathquill — comment on commit 74bb48343ed0b72556b894a073c7ea437fc2e6d0
File:  line 0
By laughinghan:

@jayferd CAN HAS HALPS? I NEEDS ALL THE HALPS

I've been obsessing over this file and I'm sick of looking at it by now, but I can't help but feel like I've either deduplicated so much it's hard to understand, or there's more that should be done, but I can't tell which.

The more I try to simplify/perfect this file, the more it smells--maybe it's just semantic satiation?

Lines 32-34 could be replaced by `Interstice.prototype.insertAt.apply(this, arguments);`, but I thought that was too much; is there an abstraction I could make here I'm missing? Or, since this isn't actually logic, we can chalk this up to JS limitations?

How do we feel about `siblingify`? At first `Group::disown` and `Group::adopt` both had slightly different copies of the body of `siblingify`, and `Node::disown` and `Node::adopt` used a singleton `Group` containing just `this`; then `siblingify` deduplicated even further. However, I have no good description of `siblingify`, since it's not a generally useful action, it can only be used in a certain way as part of something else, which smells. So maybe we liked the singleton deduplication better?

How do we feel about the `siblingify` `self` argument? That's purely to reduce line count, since the caller's next step is always to `return this`, is that dumb?