Page MenuHomePhabricator

Refactor Context::getSnakGroup
Closed, ResolvedPublic


Context::getSnakGroup will need some refactorings to split up the statements by their separators. No idea yet what that’s going to look like.

Event Timeline

Okay, I think the new interface could be:

  • getSnakGroup is renamed to getSnakGroups.
  • It gains an additional parameter, a list of property IDs (possibly empty): the separators.
  • It returns not a list of snaks, but a list of SnakGroup objects. SnakGroup is a new class holding two lists of snaks: the group itself (sibling statement main snaks, other qualifiers, etc.), and the separators (the qualifiers in case of statement contexts, empty everywhere else).

The checkers can then check the cardinality of each SnakGroup returned and report an error for any group with incorrect cardinality, using the properties of the separators in the error message.

Technically, I don’t think the checkers actually need all of this information: it would probably be enough to return a list of snak lists (Snak[][]) from getSnakGroups. But I think this will be easier to understand, test and debug if we introduce a dedicated class for the results.

Well, wait a second. Not only do the checkers not need the separating snaks… I don’t think they really need the other snak groups (other than the one that the current snak is in) either. So really – do we need to change the return type at all? Can’t we just keep it at returning the current snak group as an array? (That probably also simplifies the implementation a lot.)

Change 431575 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add separators parameter to Context::getSnakGroup()

Change 431575 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add separators parameter to Context::getSnakGroup()

Vvjjkkii renamed this task from Refactor Context::getSnakGroup to lsdaaaaaaa.Jul 1 2018, 1:12 AM
Vvjjkkii reopened this task as Open.
Vvjjkkii removed Lucas_Werkmeister_WMDE as the assignee of this task.
Vvjjkkii triaged this task as High priority.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed subscribers: gerritbot, Aklapper.
CommunityTechBot raised the priority of this task from High to Needs Triage.Jul 3 2018, 1:59 AM