CentreForDigitalHumanities / CentreForDigitalHumanities/readit-interface
Render-placement antipattern
- Dominant language
- TypeScript
- Stars
- 1
- Forks
- 0
- PR merge metrics
- No merged PRs in 30d
Description
The older parts of our code base, including the inner mechanisms of the explorer view, still contain lines that go like this:
```js
aView.render().$el.appendTo(someElement);
```
which is understandable, since this pattern occurs a lot in online example code. Which, in turn, is understandable because it is a nice oneliner that demonstrates three view idioms in one go: rendering, chaining and placement. In essence, however, this particular combination of those idioms is an antipattern.
As a rule of thumb, rendering should happen when the underlying data change, while placement happens when the context changes (such as in route changes or when a parent view renders). These events almost never coincide.
Generally, a line that follows this antipattern is motivated by a context change. This indicates that the `.render` method is being invoked for the wrong reason.
By itself, this wouldn't have to be too bad; the `.render()` method is supposed to be _idempotent_, and calling it in one place for the wrong reason doesn't strictly preclude _calling it elsewhere for the right reason_. In the best case, it's just a bit inefficient. Of course, I'm putting those words in italics because in reality, the opposite tends to be true. As with any antipattern, lines like the above are a form of code smell.
Antipatterns have a tendency to cause bugs when combined. #292 and #366 largely consisted of solving various forms of view brittleness, that resulted from a combination of the render-placement antipattern and poor subview management (#230); most commonly, failing to update when data change, breaking when rendering before data are complete, or breaking when rendering a second time. In complex composite views, the superfluous rendering can also cause noticeable slowdowns.
This story illustrates really well how improving code quality can prevent bugs already before they happen.
I'm creating a dedicated ticket for this issue now, because I just found another instance of it and I finally found myself able to express it as clearly as I did above. I learned something, and I hope @BeritJanssen and @JeltevanBoheemen can learn with me. Also, we can refer to this issue in other issues and our commit messages every time we encounter an instance of the render-placement antipattern.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.