We recently had to work around the method MooseGroupRuntimeStorage>>remove:ifAbsent:
And found its intention somehow not clear.
We understand that entities are stored into 3 different type of collection:
When we remove an element from the group storage, we want to remove the element from each of the 3 collections.
A tricky case is for byName dictionary:
in some cases, an element may not have been registered to the byName dictionary because it does not have a unique moose name (see methods #updateCacheFor: #hasUniqueMooseNameInModel)
The current implementation of #remove:ifAbsent: does not really consider this specific case.
As soon as an element is not found in the byName dictionary, the exception block is executed.
More generally, we could have expected a distinction between
- inconsistent absence of an element in one of the collection. This would raise an error.
- consistent absence of an element in all the collection. This would execute the exception block.
With this consideration, the method could be something like:
remove: anElement ifAbsent: exceptionBlock
| key group |
elements remove: anElement ifAbsent: [ ^exceptionBlock value ].
key := anElement class.
group := byType
ifAbsent: [ OrderedCollection new ].
ifAbsent: [ self error: 'Internal storage inconsistency' ].
anElement hasUniqueMooseNameInModel ifTrue:
[ "In theory, objects are registered under their mooseName,
however some objects are still registered by their name
if #resetMooseName was not used when needed"
key :=anElement mooseName asSymbol.
byName at: key ifAbsent: [ self resetMooseNameFor: anElement ].
ifAbsent: [ self error: 'Internal storage inconsistency' ] ].
What do you think ?