20323, "lydia-duncan", "Map module - should we rename the set method?", "2022-07-28T21:21:55Z"
The current set method on the map type is implemented like this:
/*
Sets the value associated with a key. Method returns `false` if the key
does not exist in the map.
:arg k: The key whose value needs to change
:type k: keyType
:arg v: The desired value to the key ``k``
:type k: valueType
:returns: `true` if `k` was in the map and its value is updated with `v`.
`false` otherwise.
:rtype: bool
*/
proc set(k: keyType, in v: valType): bool {
_enter(); defer _leave();
var (found, slot) = table.findAvailableSlot(k);
if !found {
return false;
}
table.fillSlot(slot, k, v);
return true;
}
I was expecting the map to be updated at k, and that the boolean would indicate whether the element already existed. However, it looks like in the discussion in this thread it was intentional that we distinguished it from the behavior of [k] = (though that's less strong of an argument, since the latter is currently not supported in a parallel context). In this comment which lists some precedent for the type in other languages, it looks like it is equivalent to Java's replace method. I think replace would be a better name for what it is currently doing.
Note that the list type also has a set method that behaves similarly, though in that case the failure is due to updating outside of the current bounds so it is a bit more expected imo. Still, for consistency, I would be willing to argue for renaming it to replace there, too.