Page MenuHomePhabricator

Break up circular dependencies between storage and view classes
Open, Needs TriagePublic

Description

The code architecture contains a bunch of circular dependencies:

  • RevisionList 🔁 RevisionListView: Each RevisionListView contains a reference to a single RevisionList. The same time, each RevisionList creates it's own RevisionListView on construction time.
  • Slider 🔁 SliderView: Each SliderView contains a reference to a single Slider object. The same time, each Slider creates it's own SliderView on construction time.

This alone is not really a problem. It becomes one because of the two ways RevisionLists are used:

  • Each time a new chunk of information is requested from the API, a separate RevisionList is created and rendered individually.
  • But there is also a RevisionList object in the Slider that is not used to render anything, but as a storage object that contains a merged copy of all revisions requested so far.

More specific issues/questions/confusion:

  • In SliderView.addRevisionsAtEnd() a new RevisionList is created by calling RevisionList.slice(). But it seems this view is not used. Instead, another RevisionListView is created in the next line and immediately rendered.
  • Is it possible the RevisionListViews inside each RevisionList are never actually used?
  • How many Slider objects can exist? Really only one?
  • How many SliderView objects can exist?

Event Timeline

Change 968233 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/RevisionSlider@master] Partly drop dependency from RevisionListView to RevisionList

https://gerrit.wikimedia.org/r/968233

Change 968233 merged by jenkins-bot:

[mediawiki/extensions/RevisionSlider@master] Partly drop dependency from RevisionListView to RevisionList

https://gerrit.wikimedia.org/r/968233