new ID Datastructure #5315
Replies: 4 comments 1 reply
-
also e.g. here:
|
Beta Was this translation helpful? Give feedback.
-
It's a deliberate design decision that the (To be continued...) |
Beta Was this translation helpful? Give feedback.
-
Regarding some of the other points:
|
Beta Was this translation helpful? Give feedback.
-
Some points worth highlighting:
|
Beta Was this translation helpful? Give feedback.
-
Hi @hendrikweisser @jpfeuffer @cbielow and others,
I stumbled over some design issues that might make adoption of the ID data structure
more difficult but could be fixed with some minor changes.
Things I found surprising:
Functions that register an entry like e.g.:
Either registers a new input file (as the name suggests) or merges in some information from
the other InputFile.
This doesn't follow the principle of least surprise as a user can't see from the code if
a new entry is added or modified.
Even if the user could see that an entry is modified (which he usually might not in a loop)
the semantics of the merge are poorly documented and at least surprising to the user.
But there are also more subtile issues.
E.g.:
First, it uses the operator+ which should only be used if it is crystal clear what happens. This is not the case and I agree here with more experienced authorities.
Second, the semantics of setting the experimental_design_id are also very implicit, hidden from
the user and not documented. Note that now e.g., the order in which the merges are performed
may influence what the final experimental_design_id will be. This likely leads too hard to find errors in the future
e.g., if order can't be guaranteed in multi-threaded programs.
Much safer would e.g. be to have a dedicated mergePrimaryFiles (and disallow overwriting of the experimental design id).
These problems also occur in the few other methods that try to merge in information and I think now would be a good time to address those.
Beta Was this translation helpful? Give feedback.
All reactions