leebyron / ecmascript-iterator-hof

Higher Order Functions on Iterators
42 stars 2 forks source link

Is flatMap really necessary? #4

Closed RangerMauve closed 7 years ago

RangerMauve commented 7 years ago

On can achieve the exact same effect by chaining map and flatten. When xstream (an Observable implementation) was being made, the creator, @staltz, omitted "flatMap" because [it was easier to understand]. I think this reasoning applies to higher order functions with Iterators.

leebyron commented 7 years ago

You're likely right on this one - simpler is better for proposals like this one.

flatten can be tricky if it implements deeper flattening or single-level flattening, I'll do some research to get the more common intention and attempt to use that to simplify and remove the need for a specific flatMap method

edevine commented 7 years ago

Please consider TypeScript users. flatten is difficult to apply type annotations to. flatMap is not:

flatMap<B>(map: (value: A) => Iterable<B>): Iterable<B>
RangerMauve commented 7 years ago

@edevine Would the approach taken in xstream not be sufficient? That repo is primarily typescript based and would have similar methods, so whatever was used there should work here.

leebyron commented 7 years ago

That depends on the depth of the flattening - the ability to flatten recursively is certainly up for debate. It can be useful, but it does add complexity. Simplifying it may also allow flatMap to become map().flatten()

RangerMauve commented 7 years ago

If flatten only does one depth at a time, one could write something like .map().flatten().flatten() if you for some reason plan to have iterators of iterators being thrown out of map

edevine commented 7 years ago

@RangerMauve xstream flatten is not typesafe. It merely returns T, and requires an explicit R which has nothing to do with anything at all.

export class Stream<T> implements InternalListener<T> {
  flatten<R>(): T {
    const p = this._prod;
    return new Stream<R>(
      p instanceof MapOp && !(p instanceof FilterMapFusion) ?
        new MapFlatten(p as MapOp<any, Stream<R>>) :
        new Flatten(this as any as Stream<Stream<R>>)
    ) as T & Stream<R>;
  }
}

Example:

(new Stream<number>()).flatten<HTMLElement>(); // => number

The only way flatten can be type safe is if it is a static function:

flatten<T>(stream: Stream<Stream<T>>) => Stream<T>;
RangerMauve commented 7 years ago

Tried to make a counter example, and now I see your point. Maybe one could cheat and just say that flatten<R>() => Iterator<R> and keep the internals not as type safe?

E.g.

const it = Iterator<Iterator<Number>>;
const flattened = it.flatten<Number>();
edevine commented 7 years ago

It looks like TypeScript can actually handle this pretty well. xstream is a poor example. I opened an issue with TypeScript to track an inference problem: https://github.com/Microsoft/TypeScript/issues/13323. However, it does seem type safe.

class MyArray<T> extends Array<T> {
    flatten<R>(this: R[][]): R[] {
        return (new Array<R>()).concat(...this);
    }
}

var a = new MyArray<string>();
a.flatten<string>(); // Compile type error

var b = new MyArray<string[]>();
b.flatten<string>(); // string[]
b.flatten(); // Inference error
edevine commented 7 years ago

tc39/proposal-observable doesn't spec combinators, but how likely is it that flatMap will become part of the spec? And if it does, should it be added here for API symmetry? For instance, flatMap is referenced here: https://github.com/tc39/proposal-observable/issues/75#issuecomment-173002685

RangerMauve commented 7 years ago

The observable proposal has purposefully been avoiding defining any operators, I'd say that it's more likely that anything defined here is going to influence whatever gets added in an Observable-hof proposal later on. Unless it somehow gets made and passed first which is cool too.

aluanhaddad commented 7 years ago

Although one can implement flatMap with map and then flatten a key symmetry is not exploitable in this approach. flatMap is a fundamental operation: you can define both map and filter in terms of flatMap

function filter(xs, predicate) {
  return xs.flatMap(x => predicate(x) ? [x]: []);
}
function map(xs, projection) {
  return xs.flatMap(x => [projection(x)]);
}

The answer to this Stack Overflow question really hits the nail on the head.

RangerMauve commented 7 years ago

@aluanhaddad That's a really good point. Is there maybe a list of operators not in the spec that would be easy for users to define using flatMap? That might be compelling reason to include it as well.

Personally, I still think that having a minimal set of operators that are different from each other would be easier for people to get their head around. But if it's true that it would provide a lot of utility for people, I can see the usefulness.

Obviously, this is just my opinion so it's up to the spec authors and TC39 to decide in the long run.

aluanhaddad commented 7 years ago

@RangerMauve technically all you need is reduce (foldLeft) but that would be awkward. Most languages that have these options on streams or on collections or on both, provide flatMap. Another nice aspect of it is that is provides a basis for generalized monad comprehension syntax in many languages.

In Scala, if your type has a flatMap method with the correct signature, then you can use it in a for-comprehension.

In C# if your type has a SelectMany operation (Where and Select are also required but trivial) you can use it in a LINQ expression.

These both, from what I understand borrow the notion from Haskell.

And there are many others.

This is an example in C# of a flatMappable option type that can be used in LINQ expressions.

[Fact]
public void SelectOptionFromNullPropagatesOptionOfNoneIntoProjection()
{
    var target = FromNullFactory<object>();
    var projected = 
        from value in target
        select 1;
    Check.ThatCode(() => projected.Value).Throws<InvalidOperationException>();
}

The source for the Option type is here