Hello Moose-dev,

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:

- elements

- byType

- byName

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 

        at: key 

        ifAbsent: [ OrderedCollection new ]. 

    group 

        remove: anElement 

        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 ].

        byName 

            removeKey: key 

            ifAbsent: [ self error: 'Internal storage inconsistency' ] ].

    ^anElement

What do you think ?

--