Page MenuHomePhabricator

Add common superclass of form and sense IDs and reduce code duplication in Store
Closed, DeclinedPublic

Description

In the Gerrit chain for T199206: Add wbleditsenseelements API action, I added quite a lot of duplicate code in Store (SenseRevisionLookup, SenseStore, SenseTitleStoreLookup) which has only very trivial differences from the corresponding Form… classes. Much of it only needs to know that a FormId/SenseId has a getLexemeId() method; parts also check against the BlankForm/BlankSense class. I think if we introduced a common superclass for FormId and SenseId, and perhaps some way to recognize BlankForm/BlankSense, we could get rid of a lot of duplicate code.

Patch-For-Review:

Details

Related Gerrit Patches:
mediawiki/extensions/WikibaseLexeme : masterDeduplicate form/sense store implementations
mediawiki/extensions/WikibaseLexeme : masterIntroduce LexemeSubEntity
mediawiki/extensions/WikibaseLexeme : masterIntroduce LexemeSubEntityId

Event Timeline

Restricted Application added a project: Wikidata. · View Herald TranscriptJul 23 2018, 3:21 PM

Change 447459 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseLexeme@master] Introduce FormOrSenseId

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

Change 447460 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseLexeme@master] Deduplicate form/sense store implementations

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

So this doesn’t save quite as much code as I’d hoped:

12 files changed, 502 insertions(+), 681 deletions(-)

I guess the refactoring involves some added lines which are trivial (the mostly emptied classes still need imports, doc comments, etc., and the new abstract methods also have doc comments) but still bloat the diff in lines? Not sure.

What do others think – is this worth it?

Change 449155 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseLexeme@master] Introduce LexemeSubEntity

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

Change 447459 merged by jenkins-bot:
[mediawiki/extensions/WikibaseLexeme@master] Introduce LexemeSubEntityId

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

Anything to do still here or can this be closed?

There are still two patches open, I’ve added a list to the task description.

Change 449155 abandoned by Lucas Werkmeister (WMDE):
Introduce LexemeSubEntity

Reason:
not happening

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

Change 447460 abandoned by Lucas Werkmeister (WMDE):
Deduplicate form/sense store implementations

Reason:
not happening

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

Lucas_Werkmeister_WMDE closed this task as Declined.Mar 26 2019, 3:27 PM