Page MenuHomePhabricator

[RFC] Add methods to EntityDocument
Closed, ResolvedPublic


Instead of deprecating and removing all usages of Entity::isEmpty, Entity::copy and Entity::clear, we should discuss if we want to add all or some of them to the EntityDocument interface.

Related Objects

View Standalone Graph
This task is connected to more than 200 other tasks. Only direct parents and subtasks are shown here. Use View Standalone Graph to show more of the graph.

Event Timeline

Bene raised the priority of this task from to Medium.
Bene updated the task description. (Show Details)
Bene added subscribers: Addshore, Ricordisamoa, Aklapper and 5 others.

I think it makes sense at least for isEmpty to add it to the interface. copy may not be in the interface, but should still be available in the implementations (Item, Property). clear is used only once in the api in wbeditentity, where one can set clear=true to clear the whole entity and add new data to it. This is currently used by the Wikidata Tours to provide a clean Item page.

Entity::clear seems a bit silly, we might as well create a new Entity objects. But isEmpty() and copy() seem quite useful - conceptually they appyl to any entity, and should be implemented for all kinds of entities, so they should be part of the interface.

  • clear should just die. If you need a new entity, create a new one. Never ever repurpose an existing one to be something else.
  • copy can be and is useful, but I'm not sure if it needs to be in an interface. Having it in the implementations is fine in my opinion.
  • isEmpty should be in the interface, in my opinion.
daniel claimed this task.

I agree with @thiemowmde, and from what I gathered from discussion at the office, this seems to be consensus. copy() is somewhat on the line, but it's rarely used anyway, except in test cases, where the concrete type is known.

Closing this RFC, since the basic question is answered.

I merged the pull request that adds isEmpty to EntityDocument. A release of the DataModel component would be needed to use that.