@sjakobi reviewed the code for NodeMap and noticed multiple mistakes.
The somewhat contrieved current definition of NodeMap came to be mostly due to drastic implementation changes in different commits (one of which required that the size was stored for O(1) access).
The semigroup instance is incorrect because it ignores that the intersection of the two maps might be non-empty
insertNM is incorrect since it assumes the element didn't previously exist
deleteNM is incorrect for the same reason
Above all, Data.Map.size is O(1) so we can just get rid of it all and use a cost-free newtype.
@sjakobi reviewed the code for
NodeMap
and noticed multiple mistakes.The somewhat contrieved current definition of
NodeMap
came to be mostly due to drastic implementation changes in different commits (one of which required that the size was stored for O(1) access).insertNM
is incorrect since it assumes the element didn't previously existdeleteNM
is incorrect for the same reasonAbove all,
Data.Map.size
is O(1) so we can just get rid of it all and use a cost-freenewtype
.