Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is flatMap really necessary? #4

Closed
RangerMauve opened this issue Jan 3, 2017 · 13 comments
Closed

Is flatMap really necessary? #4

RangerMauve opened this issue Jan 3, 2017 · 13 comments

Comments

@RangerMauve
Copy link

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
Copy link
Owner

leebyron commented Jan 3, 2017

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
Copy link

edevine commented Jan 4, 2017

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
Copy link
Author

@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
Copy link
Owner

leebyron commented Jan 4, 2017

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
Copy link
Author

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
Copy link

edevine commented Jan 6, 2017

@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
Copy link
Author

RangerMauve commented Jan 6, 2017

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
Copy link

edevine commented Jan 6, 2017

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: microsoft/TypeScript#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
Copy link

edevine commented Jan 7, 2017

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: tc39/proposal-observable#75 (comment)

@RangerMauve
Copy link
Author

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
Copy link

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
Copy link
Author

@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
Copy link

aluanhaddad commented Jan 9, 2017

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants