tc39 / proposal-collection-methods

https://tc39.github.io/proposal-collection-methods/
Other
172 stars 8 forks source link

#reduce, #map and #join violate their Array intuitive meaning #10

Open c69 opened 6 years ago

c69 commented 6 years ago

While filter, find, some or every play nicely with Set, and provide useful convenience. Map, reduce and join have problems, which (in my point of view) warrant their exclusion from proposal.

  1. Array.map always produces array of same length. Set.map could potentially reduce the size of the set.

    (new Set([1, 2, 3]).map(v => 1); // set of (1)
  2. Array.reduce is going from left to right, applying functor in order. Set.reduce will go by the "unique insertion order", which is not obvious and you cannot control (or update) it, as there is no #sort method on Set. Also, #reduce is not required to return same type as original collection, even more - aggregate is often desired to be of different type.

  3. Same as reduce, Array.join is joining elements in order. And "h,e,l,l,o" is very different from "o,e,l,h,l". Join returns string.

Suggestion: remove set#reduce and set#join methods - as the use cases involving them are trivially solved with [ ..mySet].reduce(fn, a)

For set#map - maybe consider different name..

Ginden commented 6 years ago

Set.map could potentially reduce the size of the set.

That's expected behaviour.

Set.reduce will go by the "unique insertion order", which is not obvious and you cannot control (or update) it, as there is no #sort method on Set.

That's why I argued that Set iteration should be entirely implementation-dependant long time ago, but it's expected behaviour to follow established convention for iterators and

Though, you shouldn't care about iteration order when you use Set.

Same as reduce, Array.join is joining elements in order. And "h,e,l,l,o" is very different from "o,e,l,h,l".

You shouldn't use Set if you care about ordering. And "I don't care about order, but I don't want duplicates" is common use case.

BTW, find depends on insertion order too.

new Set([1, 2, 3, 4]).find(e => e %2)
c69 commented 6 years ago

"I don't care about order, but I don't want duplicates" is common use case.

indeed. Still what to do with the Set once we got rid of duplicates is up to developer.

find depends on insertion order too

true.

But back to join and reduce.

Main point is that all of them are redundant in Ecmascript.

In contrast to very needed union, intersect and difference, or more general filter, which have type of (this: Set) => Set. The methods I oppose to (join and reduce) all have the signature of (this: Set) => any, with emphasis on any. And my question is

[...mySet].find(v => v % 2); mySet.find(v => v % 2);

[...mySet].reduce((a,v) => av, 1); mySet.reduce((a,v) => av, 1);

[...mySet].join('|'); mySet.join('|');


For `some`, `every` and `find`, the justification could be that because those three do not need to always traverse the full collections, their performance could be better on Set, compared to use of intermediate array (as spread has to put all he elements in). 

And extra points against `join` below.

## Violated assumptions:
- `Array.join` is a dual of `String.split`. But there is no `String.splitToSet` and there should not be.

## Inconsistency: 
- `Array.join` is needed to "turn array elements into a string with custom separator maintaining their order", being something like configurable brother of `Array.toString`. But `mySet.toString()` and `JSON.stringify(mySet)` behave quite differently (and useless) from their Array counterparts and `Array.join`:
```javascript
let a = new Set([1, '2', {x: 3}, [4]]); 
[...a].toString(); // "1,2,[object Object],4"
[...a].join(); // "1,2,[object Object],4"
JSON.stringify([...a]); // "[1,"2",{"x":3},[4]]"
a.toString(); // "[object Set]"
JSON.stringify(a); // "{}"

Limited usefulness:

TL;DR if those methods are added, we gain almost nothing for the effort spent

Ginden commented 6 years ago

we already have a reasonably cheap (in both performance and syntax sense)

Well, compare this:

return set
    .map(fn1)
    .filter(fn2)
    .map(fn3);

To this:

return new Set([...new Set([...set].map(fn1)).filter(fn2)]).map(fn3);

Conversion Set => Array => Set isn't reasonably cheap in syntax sense.

The methods I oppose to (join and reduce) all have the signature of (this: Set) => any, with emphasis on any.

.join is always (this: Set) => string.

If its members are not primitives, mySet.join() will return quite a useless string

Array.prototype.join behaves the same way.

And such behaviour of Array.prototype.toString is a mistake that can't be fixed due to backwards compatibility.

c69 commented 6 years ago

Your example of filter and map illustrates well why we need new methods with (this: Set) => Set. Maybe i was not clear enough, but i wholehartly support filter and map (with maybe different name).

But once discussion focuses on join and reduce, then syntactic cost becomes much lighter. Because join terminates the chain (by returning string), and reduce does not need extra parenthesis to return new Set, e.g.:

[...mySet].reduce ((a,v) => a.add(magic(v)), new Set()).add(1).delete(2).size;

Now, if you need chained reduce calls for some reason.... things become uglier:

[...[...mySet].reduce ((a,v) => a.add(magic(v)), new Set()).add(1)].reduce (f2).delete(2).size;

I can argue, though, that chained reduce is a rare case, and its hard to imagine real example which cannot be converted to [...mySet.filter(f1).filter(f2)].reduce(g)

But yes, - nested spread blocks are not pretty, just wandering how frequent such cases are.

c69 commented 6 years ago

After reading more threads, with valid points about being able to conver their Sets (and Maps) to anything...

How about Set.fold (and i guess Map.fold) with nondeterministic order and type signature? :


function fold(this: Set, accumulator: any, value: any): any { 
}
ljharb commented 6 years ago

I'm still not understanding what confusion there would be with "reduce" - fold and reduce do the same thing.

As for the "index", sets don't have an index, so likely just like forEach, the callback would either get (value, value) or just (value).

Assumptions about order are correct - Set and Map in JS are inherently ordered, by insertion order.

c69 commented 6 years ago

@ljharb maybe you are right, and i am overdramatizing the "imaginary" developer's confusion.

So let's leave reduce out of the discussion in this thread. And just focus on the join.

dead-claudia commented 5 years ago

@c69 .join(sep) is basically .reduce((acc, value) => acc == null ? value : acc + sep + value, null). Any deviation here would be unexpected, and if you provide .reduce, .join just logically follows from it.

IMHO, the only thing here that really raises questions is .map. I'm not convinced it has enough of a use case to warrant standalone inclusion on Set.prototype.