montagejs / collections

This package contains JavaScript implementations of common data structures with idiomatic interfaces.
http://www.collectionsjs.com
Other
2.09k stars 185 forks source link

Map collection.add(value, key) has illogical parameter ordering #101

Closed zobalogh closed 9 years ago

zobalogh commented 9 years ago

Your current Map implementation has an add function that has the illogical (value, key) parameter ordering. I believe the ordering is wrong.

Unfortunately the API has been released as is so a change to the API without regression is not possible.

Your documentation (http://www.collectionsjs.com/method/add-value-key) mentions the "gotcha" ("that does not have an obvious reason to exist") but that doesn't help the casual user who just sees the main page (http://www.collectionsjs.com/map).

It took me a few hours today to realise that a bug in my code is because the add arguments are unusually ordered within your API.

add() and set() should have distinctive meaning for maps. Add should add a new element, set should refresh the value given for an existing key.

Raising this issue against your code instead of the docs but I believe a change is needed for both the docs and the API.

I suggest deprecating "add(value, key)", adding a temporary addKV(key, value) function and also warning the user about this in the docs with bold/red text.

I'm willing to help and make such changes to your API and or documentation.

Please let me know if I'm completely wrong and if there was an engineering decision behind your unusual ordering.

Thanks!

kriskowal commented 9 years ago

set(key, value) should suffice for your usage. There is no method on a map that can create an additional entry when one for the given key already exists. add(value, key) and set(key, value) have the same behavior. The add(value, key?) method exists on all collections so that various generic methods implemented in generic-collection.js can use a single interface to update collections of any type. The add method is implemented differently on each type of collection, many of which ignore the key.

In short, the add method of a map exists only as an implementation detail of generic collection methods.