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 created this task.Aug 25 2015, 11:55 AM
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.
Bene added a comment.Aug 25 2015, 11:58 AM

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 closed this task as Resolved.Aug 31 2015, 12:35 PM
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.

daniel added a comment.EditedAug 31 2015, 12:36 PM

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

aude moved this task from Review to Done on the Wikidata-Sprint-2015-08-18 board.Aug 31 2015, 12:56 PM