applitopia / immutable-sorted

This is an extension of Immutable.js that provides sorted collections SortedMap and SortedSet. The current implementation is using highly optimized B-tree memory structure.
http://applitopia.github.io/immutable-sorted/
Other
29 stars 6 forks source link

Feature request: get-and-delete operation for SortedMap #5

Open abonander opened 6 years ago

abonander commented 6 years ago

I just today realized that Immutable.js doesn't provide an operation for this, though I really only need it for SortedMap. I would like a method on SortedMap that removes and returns the value under the given key as well as the updated map. In Flow the function prototype is straightforward:

getAndDelete(key: K): [?V, SortedMap<K, V>] {}

let [val, map] = map.getAndDelete('key');

Apparently in Immutable.js this can be done by returning null from the closure passed to Collection.update() (though this is undocumented) but SortedMap doesn't appear to support this; it just updates the value as null. I know this can be done with get() then delete() but I'd like to avoid the redundant lookup if possible.

applitopia commented 6 years ago

Thank you for your suggestion, this would would definitely be more efficient than two separate calls. It would be nice to have it implemented across all the collections (List, Set, Map, OrderedSet, OrderedMap, SortedSet, SortedMap)

Please note, the get function includes optional notSetValue argument to distinguish the missing key in the map from the key having unusual values like null, undefined, {}, etc.:

get<NSV>(key: K, notSetValue: NSV): V | NSV;

Would it make sense to include this additional argument into getAndDelete()?

abonander commented 6 years ago

Would it make sense to include this additional argument into getAndDelete()?

It certainly wouldn't hurt though returning any falsey value is sufficient for my use-case. I'm not sure how default parameters interact with type parameters, though; is this valid syntax?

getAndDelete<NSV = ?V>(key: K, notSetValue: NSV = null): V | NSV {}
abonander commented 6 years ago

As for merging it upstream, I figure it would be easier to prototype here first before proposing it for Immutable.js. Implementing it on SortedMap would be enough for me either way.