https://bugzilla.wikimedia.org/show_bug.cgi?id=64873
Thiemo Mättig <thiemo.maet...@wikimedia.de> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |thiemo.maet...@wikimedia.de
--- Comment #1 from Thiemo Mättig <thiemo.maet...@wikimedia.de> ---
I created a discussion item for this a few weeks ago and started collecting
oddities:
https://github.com/wmde/WikibaseDataModel/issues/73
Now that we are actually working with the Fingerprint classes in the ChangeOps
it turns out to be as unpleasant as I was afraid of. Sorry. This is really not
meant to be personal. The initial attempt was nice and well-meant: Use objects
for everything. Make most immutable. This sounds nice from a design perspective
but makes doing real stuff (like checking if changing a label is going to work
before actually changing it, like it is done in the ChangeOps) complicated.
There are multiple problems:
The naming conventions inside the Fingerprint classes are just not consistent
and self-explaining. For example, methods are using the word "descriptions"
inconsistently. It can mean anything, an array of strings, an array of
Description objects, an array of Term objects or a List object (what I
described as "reimplementing arrays" before).
Some methods that are named after classes return strings and string arrays
while others that sound like they return strings return objects.
See https://github.com/wmde/WikibaseDataModel/pull/86 for a discussion about
the naming convention.
The currently most unpleasant use case is when a ChangeOp wants to check if
modifying a Fingerprint is going to work without actually changing the object.
It needs a copy. But how to do this? If you want to avoid PHP's clone you need
to create a very complicated method that iterates all labels, terms and aliases
in all alias groups, avoids re-using any of the nested objects and instantiates
new objects from the values.
There are methods to get string arrays of labels and descriptions but none to
create objects from such arrays.
The most straightforward interface would, in my opinion, look like this:
class Fingerprint {
string[] getLabels();
setLabels( string[] );
string[] getDescriptions();
setDescriptions( string[] );
array[] getAliases();
setAliases( array[] );
string getLabelForLanguage( string );
setLabelForLanguage( string, string );
string getDescriptionForLanguage( string );
setDescriptionForLanguage( string, string );
string[] getAliasesForLanguage( string );
setAliasesForLanguage( string, string[] );
}
All arrays are associative arrays with the language codes as keys.
Internally the class can still use Term objects, List objects and so on. If the
interface is clean and easy to use that doesn't really matter in the end.
An other attempt is to make the Fingerprint classes partially do what is
currently done in the ChangeOp validation. Example: $fingerprint->setLabel( ''
) raises an exception if a hard constraint is validated. Then we don't need any
methods to de- and reconstruct a Fingerprint from the outside. But I'm pretty
sure that's not the way we want to go.
--
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
