Yomguithereal / mnemonist

Curated collection of data structures for the JavaScript/TypeScript language.
https://yomguithereal.github.io/mnemonist
MIT License
2.26k stars 92 forks source link

MultiMap.from broken #174

Open andreasdamm opened 2 years ago

andreasdamm commented 2 years ago

When using MultiMap.from to create a MultiMap from an iterable of tuples or one-attribute-objects, the index of the item from the iterable is used as the key into the map instead of the first member of the tuple, or the attribute name of the one-attribute-object.

MultiMap.from([['a', 'aa']], Set) currently results in Map(1) { 0 => Set(1) { [ 'a', 'aa' ] } } MultiMap.from([{a: 'aa'}], Set) currently results in Map(1) { 0 => Set(1) { { a: 'aa' } } }

The desired output should be Map(1) { 'a' => Set(1) { 'aa' } }

Yomguithereal commented 2 years ago

Hello @andreasdamm,

MultiMap.from actually works as intended because it follows the standard used throughout the library regarding mixed iteration over potentially key-valued containers. But it does not follow JS Map constructor's rationale indeed because it does not make sense in a broader perspective. This said I can add another static method called #.fromEntries tailored to handle this specific use case as it makes sense in relation with JS Map if you want. It would handle 2-tuples as you wish it to. But it would not handle single attribute object as it does not make sense and because a JS Map does not understand those either.

andreasdamm commented 2 years ago

Would the declaration file for MultiMap () multi-map.d.ts) have to be changed to reflect the intended behavior? Right now it is

interface MultiMapConstructor {
  new <K, V>(container: SetConstructor): MultiMap<K, V, Set<V>>;
  new <K, V>(container?: ArrayConstructor): MultiMap<K, V, V[]>;

  from<K, V>(
    iterable: Iterable<[K, V]> | {[key: string]: V},
    Container: SetConstructor
  ): MultiMap<K, V, Set<V>>;
  from<K, V>(
    iterable: Iterable<[K, V]> | {[key: string]: V},
    Container?: ArrayConstructor
  ): MultiMap<K, V, V[]>;
}

which indicates that the first member of the tuple becomes the key, and the second one the value.

Yomguithereal commented 2 years ago

The type declaration is probably wrong indeed. I have better types suited to this kind of thing in obliterator. I will fix that and add #.fromEntries in the meantime.

andreasdamm commented 1 year ago

There still doesn't appear to be a good way to create a MultiMap from a list of entry tuples, other then creating an empty map and then iterating through the tuples in a loop calling MultiMap.set. How would oblitarator help?