Hey,
Now the design issues of EntityId have been fixed, it's Entity's turn :)
(Note: this is about domain layer implementation details. Not considering changing anything visible to the user here.)
While working on the QueryEntity together with addshore, we ran into a number of issues with the current implementation of Entity. The main problem is that Entity objects are constructed from their internal "serialization". The constructor, which is marked as protected, takes in this "serialization" (in array form). This is rather awkward, consider how we now typically construct a Property:
$property = Property::newEmpty(); $property->setDataTypeId( $id );
(We also have a static newFromDataTypeId which wraps this.)
There is a bunch of code that assumes one can create empty Entity objects. Esp in tests. I now think it was a mistake to allow this at all for Property, which should not be constructed without a dataTypeId. The same goes for QueryEntity, which should not be constructed without a Ask\Query. It'd be much nicer if people could just use the constructors of the objects and have these enforce the list of required parameters. They'd just take the actual objects and not serializations.
$property = new Property( $id ); $queryEntity = new QueryEntity( $askQuery );
And since these things are enforced, one now gets back a string when calling getDataTypeId, and a Ask\Query when calling getQueryDefinition, rather then either that type or null.
Serialization and deserialization code can also go into dedicated service objects. This is already done in QueryEntity, which is using the same serializers as the ones the web API will use, saving us implementation of a second format, which would not be of much help here anyway (I'd save some disk space...).
There is also a lot of room to be more strict about things. Right now you can happily construct a Entity that has ints as aliases, or as language code for labels. On top of that, there are currently still TODOs from the first months of the project in Entity related to normalization and handling of duplicates. We might want to clearly define responsibilities at this point :)
Oh and, of course Entity, Item, Property and Query each should go into their own git repo.
Any objections or concerns about the above rambling?
There lately has also been some talking about doing things with Entity that we did not consider before. Such as entities that contain other entities. Is there a list of such thoughts? If not, lets compile one here so these can be held into account.
Cheers
-- Jeroen De Dauw http://www.bn2vs.com Don't panic. Don't be evil. ~=[,,_,,]:3 --
Hey,
Fun fact, after the EntityId refactoring, Entity is now the highest risk class in the component according to PHPUnit.
- Entity (200) - HashArray (71) - Item (45) - EntityDiff (41) - Claims (27) - ObjectComparer (23) - EntityId (22) - SnakObject (21) - Claim (19) - Reference (16)
Cheers
-- Jeroen De Dauw http://www.bn2vs.com Don't panic. Don't be evil. ~=[,,_,,]:3 --
Hey,
I got asked what "high risk" in my last email means.
These results are using the CRAP metric [0] to determine the CRAPiness of the code (which is indicated by the number in brackets). You can have PHPUnit generate this by running the following command in the root directory of WikibaseDataModel and clicking "dashboard" on the generated index.html page: phpunit --coverage-html /some/path
[0] http://googletesting.blogspot.de/2011/02/this-code-is-crap.html
Cheers
-- Jeroen De Dauw http://www.bn2vs.com Don't panic. Don't be evil. ~=[,,_,,]:3 --
Am 31.08.2013 03:03, schrieb Jeroen De Dauw:
Hey,
I got asked what "high risk" in my last email means.
These results are using the CRAP metric [0] to determine the CRAPiness of the code (which is indicated by the number in brackets). You can have PHPUnit generate this by running the following command in the root directory of WikibaseDataModel and clicking "dashboard" on the generated index.html page: phpunit --coverage-html /some/path
[0] http://googletesting.blogspot.de/2011/02/this-code-is-crap.html
I suggest to have this generated regularly and put it online somewhere (maybe on wikidata-docs.wikimedia.de).
-- daniel
wikidata-tech@lists.wikimedia.org