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

List method flatmap is inappropriately discouraged #1428

Closed
grimnebulin opened this issue Jul 29, 2017 · 23 comments
Closed

List method flatmap is inappropriately discouraged #1428

grimnebulin opened this issue Jul 29, 2017 · 23 comments
Labels
docs Documentation issue (primary issue type) update part of "docs" - indicates this is an update for an existing section; rewrite, clarification, etc.

Comments

@grimnebulin
Copy link

The file List.pod6 contains some relatively new verbiage which discourages the use of the flatmap method in favor of calling map followed by flat. Using flatmap is called "bad practice" and "confusing." But flat-mapping is a common technique in many programming languages, and there seems to be no particular reason to avoid in in Perl 6. Therefore, I would request that this warning be removed.

@AlexDaniel
Copy link
Member

AlexDaniel commented Jul 29, 2017

Please see RT #130520 and say if you still think that the warning should be removed :)

Maybe to resolve this issue we should simply add a link to the ticket.

@b2gills
Copy link
Contributor

b2gills commented Jul 29, 2017

Basically .flatmap(…) is the same as .map(…).flat or flat(.map(…)) but it looks to be special in the same way as .deepmap(…) and .duckmap(…) are. Neither of which can be split into two method calls because they exist for iterating over their values in a special way.

So there is only few possible reasons to keep it:

  1. It exists already
  2. It is significantly faster than the alternative (not very likely, but not tested either)
  3. It is much easier to type or remember (it is only slightly easier to type)
  4. It can be used to help make code much easier to read/understand (only slightly)

Perhaps there are other reasons I'm not thinking of, but it seems the main reason to keep it is that it exists. If it didn't exist already, it would not be an accepted addition to the language.

This is just a quick summary of arguments against it in RT #130520

@AlexDaniel
Copy link
Member

It is being used in some modules, but very likely some of these cases are old, bit-rotted and were relying on pre-glr behavior.

In theory we can wipe out its usage with less than 30 pull requests. Actual users are unlikely to use it because the documentation says not to.

@coke coke added big Issue consisting of many subissues docs Documentation issue (primary issue type) and removed big Issue consisting of many subissues labels Jul 30, 2017
@zoffixznet
Copy link
Contributor

zoffixznet commented Jul 31, 2017

Basically .flatmap(…) is the same as .map(…).flat or flat .map(…)

Note that the two alternatives you provide aren't equivalent, which is I think the core issue with .flatmap is. It suggests a .flat.map call (or some more elaborate feature), while in reality it's just a .map.flat

<Zoffix__> m: [[<a b>], [<c d>]].map(*.List).flat.perl.say
<camelia> rakudo-moar 7fdbb4: OUTPUT: «("a", "b", "c", "d").Seq␤»

<Zoffix__> m: [[<a b>], [<c d>]].flat.map(*.List).perl.say
<camelia> rakudo-moar 7fdbb4: OUTPUT: «(("a", "b"), ("c", "d")).Seq␤»

<Zoffix__> m: [[<a b>], [<c d>]].flatmap(*.List).perl.say
<camelia> rakudo-moar 7fdbb4: OUTPUT: «("a", "b", "c", "d").Seq␤»

Here's a more destructive example, considering .flat breaks up uncontainerized hashes into pairs. Note the return value is a Seq of Pairs:

<Zoffix__> m: sub make-a-hash { %(:42foo, le-hash => $^foo) }; dd (<a b c>, <d e f>).flatmap: *.&make-a-hash
<camelia> rakudo-moar 7fdbb4: OUTPUT: «(:le-hash($("a", "b", "c")), :foo(42), :le-hash($("d", "e", "f")), :foo(42)).Seq␤»

@zoffixznet
Copy link
Contributor

I listed .flatmap for deprecation in 6.d and removal in 6.e: Raku/6.d-prep@2cc614b

@AlexDaniel
Copy link
Member

Cool. i think we can close this issue if we change the docs to say that it will be deprecated in 6.d (instead of saying that flatmap is a bad practice). It may be too early for that, but still.

Basically .flatmap(…) is the same as .map(…).flat or flat .map(…)

Note that the two alternatives you provide aren't equivalent

But they are? flat $_.map(…). flat .map(…).

@zoffixznet
Copy link
Contributor

But they are? flat $_.map(…). flat .map(…).

Ah, I misread the second one wasn't a method chain.

@luben
Copy link

luben commented Aug 1, 2017

I think it's really bad decision to deprecate it. It's one of the 2 operations needed for defining monad (it's also known as "bind' or ">>=" in haskell) and is common name in a lot of languages (even Java has it nowdays). Yes it can be replaced with map().flat but this comes at the cost of another container allocation and breaking expectations.

@AlexDaniel
Copy link
Member

AlexDaniel commented Aug 1, 2017

@luben what container allocation? Can you clarify?

And also, what expectations?

@zoffixznet
Copy link
Contributor

this comes at the cost of another container allocation

Wouldn't shesh optimize that away? And in any case, at least the current rakudo implementation does just that; two method calls.

breaking expectations

But that's the key issue with the method. There's no easy-to-discern expectation. .flat.map(&foo) is different from .map(&foo).flat, the method name suggests the behaviour is the former, but the actual implementation is the latter.

@luben
Copy link

luben commented Aug 2, 2017

Regarding the allocation, m.map(f).flat decomposes into { tmp c = m.map(f); tmp.flat }, so the tmp is new container allocation that is avoided by flatmap.

Even if spesh starts optimizing this combination of method calls over lists I don't expect it to be able to do the same in the generic case.

Regarding the expectations of people coming from other languages, flatmap is common name, e.g. java, scala, swift, rust, JS functional libraries, etc.

@toolforger
Copy link

toolforger commented Aug 2, 2017 via email

@jonathanstowe
Copy link
Contributor

@AlexDaniel I'll take a look at the modules I'm in control of. I've just altered https://github.com/jonathanstowe/Chronic and will take a look at https://github.com/sergot/http-useragent this evening, so on my part I'm for it.

@zoffixznet
Copy link
Contributor

zoffixznet commented Aug 2, 2017

Regarding the allocation, m.map(f).flat decomposes into { tmp c = m.map(f); tmp.flat }, so the tmp is new container allocation that is avoided by flatmap.

I don't think we create any temporary containers to store intermediary results.

Even if spesh starts optimizing this combination of method calls over lists I don't expect it to be able to do the same in the generic case.

I was talking about the generic case and I think rakudo implementation already does it. But even if it didn't, we're not adding a whole 'nuther method to the language for a theoretical saving of a single container allocation in implementation X.

@AlexDaniel
Copy link
Member

I don't know much about this, so bear with me, but… You've been talking about intermediate lists and things getting optimized away, but I don't really see how this corresponds to what we have. Both map and flat return a Seq, so why would there be any temporary list? Is there any case when this actually happens?

For example:

({my $x = ++$; say generate $x; $x} … ∞).map({say map $_; $_ + 1}).flat[^5].say

Result:

generate 1
map 1
generate 2
map 2
generate 3
map 3
generate 4
map 4
generate 5
map 5
(2 3 4 5 6)

@b2gills
Copy link
Contributor

b2gills commented Aug 6, 2017

There is no way that .flatmap(…) can be made to be more than slightly faster than .map(…).flat because .flat descends deeply into non-itemized lists. So it will always have to look at what the user code in the .map(…) returns and descend into it.

If it can be made to be significantly faster, we can add a .flat method to the result of .map(…) that transforms it into a flatmap.

So if .flatmap(…) can be made faster or more memory efficient, .map(…).flat should get the same enhancement.

Basically just because it is a good idea in Haskell doesn't mean it is a good idea in Perl 6.

@toolforger
Copy link

toolforger commented Aug 6, 2017 via email

@smls
Copy link
Contributor

smls commented Aug 19, 2017

@toolforger

The commonality among other languages with strong list/sequence and functional support is more convincing (to me anyway).

Different names, syntaxes, and conventions are ideal in different languages.

Larry Wall has repeatedly talked about regretting certain C-isms that he had to put in Perl, in order for the language to not be rejected by the programming community of that time as "insufficiently C-like".
Breaking with that habit, i.e. prioritizing internal consistency and elegance over superficial commonality with other languages, has been an important aspect of the Perl 6 design process.
That's why e.g. Perl 6 's ternary operator ended up as ... ?? ... !! ... instead of ... ? ... : ....

Let's compare flatmap between Perl 6 and other languages:

  • Benefits added:
    In many other languages, flatmap has one or both of the following benefits compared to chaining map and flat:
    • Less clutter: flatmap(...) is neater than flat(map(...)) in languages where those parens are mandatory. But in Perl 6 they're not. Both the functional version (flat map ...) and the method version (map(...).flat) would only save a single, unintrusive character (space or dot) when using flatmap.
    • Performance: In languages where map has to fully allocate and eagerly generate its output list before passing it to flat, a built-in flatmap can double the memory-efficiency by skipping the intermediate list. But in Perl 6, map returns a Seq, which is basically an iterator and is consumed lazily by the subsequent flat. So this benefit, too, disappears in Perl 6.
  • Harm caused:
    • Confusion:
      In languages where functions are used for everything, it is reasonably easy to guess and remember that flatmap(...) is short for flat(map(...)) - it's the order in which you read it.
      But in Perl 6, these things also exist as methods, and I indeed the method forms seem to be more popular. It's quite easy to get confused and think that .flatmap(...) means .flat.map(...), when it actually means .map(...).flat. The fact that the similar-sounding .deepmap and .duckmap methods behave fundamentally differently makes it even worse.

TL;DR: flatmap is useful and benign in certain other languages, but virtually useless and somewhat harmful in Perl 6. Ergo, it makes sense to deprecate it and discourage it in the documentation until it is gone.


PS: People who want to code in Perl 6 as if it was Haskell, can simply write flat map with the space. No big deal. Or if they really want to write flatmap, they can define it in their own code as easily as:

my &flatmap = &flat o &map;

@toolforger
Copy link

@smls I'd usually agree, but you forgot an important benefit:
Universality: flatmap seems to be such a universally common pattern that it's recurring in different languages, and sometimes even under different names.

From that derives:
Confusion reduction: We'd have a single name for the same operation, not the reinvent-the-wheel names that have crept into other languages.

So the picture is clear if and only if we ignore some benefits.

There's also the following harm:
Non-elementary function added: Classes with non-elementary functions are a code smell. They can make it more difficult to write subclasses/subtypes, because you need to know whether flatmap should just be the standard composition &flat o & map or something else. I don't worry too much about subclass authors, but I do worry about callers who will be unsure whether all subclasses Do the Right Thing.

I don't find any of the arguments overly compelling, except the one that flatmap seems to be so universally used that people even reinvent it.
If somebody can argue that the alternate names came into existence just because the various frameworks didn't properly encapsulate lists in the base class, then that argument becomes much less compelling I think.

@toolforger
Copy link

P.S. I hope I made it clear that I'm definitely not after writing Haskell in Perl6.
I do not think that that's a serious issue though; people who want to write Haskell are coding in Haskell and won't care much about what Perl6 does or does not.

@b2gills
Copy link
Contributor

b2gills commented Aug 19, 2017

I'd usually agree, but you forgot an important benefit:
Universality: flatmap seems to be such a universally common pattern that it's recurring in different languages, and sometimes even under different names.

From a certain viewpoint we already have it, except with a space.

my @a = flat map *.name, @b

Confusion reduction: We'd have a single name for the same operation, not the reinvent-the-wheel names that have crept into other languages.

This is not a good argument, we renamed length to chars and elems, and renamed the C-style for loop to loop. The reasons were to reduce confusion, and promote iterating over a sequence/range instead. (less error prone) What makes flatmap so special that we wouldn't consider renaming it? I suggest that if we keep it to rename it to flattened-map to make it more clear that it is flat map Callable, Iterable and not Iterable.flat.map(Callable) (note that the first flattens after, and the second flattens first)

Non-elementary function added: Classes with non-elementary functions are a code smell. They can make it more difficult to write subclasses/subtypes, because you need to know whether flatmap should just be the standard composition &flat o & map or something else. I don't worry too much about subclass authors, but I do worry about callers who will be unsure whether all subclasses Do the Right Thing.

Apparently you don't realize this is almost an excellent reason to remove it.

Anybody could come along and add a flatmap method to their class that does something completely different. Which would catch people out who thought it was the same as .map.flat which it is for every other class by default. For .map.flat to do something unexpected you have to change your class to return something other than a normal Seq from .map.

Actually that is even more reason to remove it, it is currently taking up a method name that a class author might want to use for something else one day. (minor point)


One of the goals in the design of Perl 6 is that it is worse to confuse an expert than a neophyte. I've been programming almost exclusively in Perl 6 for so long that I basically have to translate other languages into it in order to understand them. I was confused when I first saw flatmap not knowing if it was .map.flat or .flat.map or even a special type of .map like .deepmap or .duckmap that does some sort of flattening behaviour. Now whether you, I, or anybody consider me to be an expert is debatable, but it does show that someone that uses Perl 6 a lot has been confused by this being in the language.

Also note that removing it couldn't confuse a neophyte, except insofar as they might be confused why it isn't there. Even then they could only be confused if they came from one of the languages that has it.


About the only reasons for keeping it are:

  1. Other languages, which have almost entirely different design and implementation goals in mind, have it.
  2. Those same languages tend to have it with the same name.
    We have already changed the names of some things from other languages to better suit our needs.
    Also we sort of needed to do that in order to combine so many ideas into one language and make them seem as if they have always belonged together.
  3. Performance
    It currently probably isn't, and most likely never will be anything more than slightly faster. It might also be slower depending on what spesh does.
  4. Less memory
    Doesn't have much if any effect on memory currently, and most likely never will.
  5. Monads‽
    We don't really have a need for monads because it isn't an exclusively functional language.
    (You could think of it as a functional language with plenty of escape hatches.)
  6. But flat-mapping is a common technique in many programming languages, and there seems to be no particular reason to avoid in in Perl 6.
    It's already in the language with other arguably less confusing ways to write it.
    So we aren't avoiding it, we are just removing this ever so slightly shorter way of writing it.

Reasons to remove it:

  1. It is only an ever so slightly shorter alias.
  2. It is potentially confusing to an expert in the language.
    It looks similar to .deepmap and .duckmap even though it isn't.
    It could be an alias to .flat.map or .map.flat or .flat.map.flat
    It is another special case that has to be remembered.
  3. It is just one more special case, in a language that has been designed to remove as many special cases as possible.
    Notice that we don't have a reversemap, headmap, tailmap, pairsmap, permutationsmap, or really any other combination of a method name on Seq combined with map.
    Almost all arguments against those also apply to flatmap.
  4. It is another source for action at a distance if anybody adds a .flatmap that does something different to one of their classes.
  5. It is taking up a method name that someone might have a use for. (minor point)
  6. It is another (minor) source for bugs in the implementation that is otherwise unnecessary.
  7. It is another feature that another implementation of Perl 6 would have to include.
  8. It is another routine that a highlighting text editor or IDE has to deal with specially.
  9. Other languages have something similar to it with the same name, even though they have a different idea about how Lists work.
    This could confuse people who use those languages into thinking they would use this method/subroutine in the same places they would in those languages.

About that last one, note that to get something that has similar memory usage to Haskell's foldl (not foldl') you could write something like:

my $sum = [\+]( 1..10 ).tail;
# (1, 3, 6, 10, 15, 21, 28, 36, 45, 55).Seq.tail

or to get closer to how it would work in Haskell

multi curry-add ( &c, $head ){
  -> &prev, $next { prev +$head + $next }.assuming( &c )
}
multi curry-add ( $head, $second ){
  -> $next { $head + $second + $next}
}
my $sum = [[&curry-add]]( 1..10 ).(0);

Rather than:

my $sum = [+] 1..10;

I can only assume that other purely functional languages would have similar disparities with how Perl 6 works with lists.

@coke coke added the update part of "docs" - indicates this is an update for an existing section; rewrite, clarification, etc. label Aug 26, 2017
@AlexDaniel
Copy link
Member

OK folks, this discussion is awesome, right. But in terms of a doc ticket, there's nothing to be done here. We have an RFC ticket to toss it out, so discouraging flatmap is appropriate, especially considering that flat .map() isn't any harder to type. Let's not have hundreds of people use .flatmap just because there's a tiny chance that it won't be removed.

I'll change the wording a little bit and close the ticket. Feel free to continue your discussion on https://rt.perl.org/Ticket/Display.html?id=130520 (if it is ever decided that this RFC is rejected, we can remove the discouraging message from the docs).

@AlexDaniel
Copy link
Member

Feel free to reopen and continue discussion here, but please stay on topic (i.e. “we should not discourage the use of a feature that is strongly RFC-ed for removal because …”).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation issue (primary issue type) update part of "docs" - indicates this is an update for an existing section; rewrite, clarification, etc.
Projects
None yet
Development

No branches or pull requests

9 participants