[Review] SE-0187: Introduce Sequence.filterMap(_:)

John_McCall · November 8, 2017, 8:25pm

You can built it out of map and filter:

  func filterMap<T>(_ fn: (Element) -> U?) -> [U] {
    return map(fn).filter { $0 != nil }.map { $0! }
  }

John.

···

On Nov 8, 2017, at 3:20 PM, Tino Heth via swift-evolution <swift-evolution@swift.org> wrote:

This is a wonderful example! But it’s an argument for a different discussion (of general usefulness of implicit optional promotion). Thanks to the optional promotion, what the closure returns is not nil, but instead is .some(nil), and that is not filtered out.

My point is: The proposed filterMap isn’t a combination of map and filter at all — or can you build it just out of map & filter, like flatMap is constructed from map & flatten?

Tino · November 8, 2017, 9:29pm

True, but it’s not what I asked for — and it should rather be called „mapFilterMapForceUnwrap“, shouldn’t it? ;-)
The thing is that people who don’t fully understand flatMap (those that are the target group for the proposal) probably just use it because „it does a transformation and filters out nil (and nil is evil)“.
Renaming the function to filterMap hardens that misconception:
As you sample shows, filterMap is not like flatMap — the latter is honest („flatten the result of map“) the other isn’t („filter the result of map“ only strips the nils, but doesn’t change the return type)

So I think it’s better to keep flatMap, or replace it with something that doesn’t pretend to be a plain filter.

···

Am 08.11.2017 um 21:25 schrieb John McCall <rjmccall@apple.com>:

My point is: The proposed filterMap isn’t a combination of map and filter at all — or can you build it just out of map & filter, like flatMap is constructed from map & flatten?

You can built it out of map and filter:

  func filterMap<T>(_ fn: (Element) -> U?) -> [U] {
    return map(fn).filter { $0 != nil }.map { $0! }
  }

Rod_Brown · November 8, 2017, 10:21pm

Kevin,

I really like your reasoning about avoiding the code churn, and finding a better way to flag something to avoid optional hoisting. I think its better solution to the problem. It also has the added benefit of kicking out to map() and getting a slightly optimised version of the same method - free speed win for devs who unknowingly used `flatMap` not realizing `map()` is the preferred solution.

I’m not sure the potential for confusion is enough to add a full new attribute flag for “Warning” though. It’s a minor win with a large relative overhead, and explaining in the deprecation warning that flat maps that act like direct maps are not recommended is reasonable enough.

- Rod

···

On 9 Nov 2017, at 5:43 am, John McCall via swift-evolution <swift-evolution@swift.org> wrote:

On Nov 8, 2017, at 1:20 PM, Kevin Ballard <kevin@sb.org <mailto:kevin@sb.org>> wrote:

On Tue, Nov 7, 2017, at 09:37 PM, John McCall wrote:

On Nov 7, 2017, at 6:34 PM, Kevin Ballard via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Tue, Nov 7, 2017, at 03:23 PM, John McCall via swift-evolution wrote:

https://github.com/apple/swift-evolution/blob/master/proposals/0187-introduce-filtermap.md

• What is your evaluation of the proposal?

This proposal is going to cause an insane amount of code churn. The proposal suggests this overload of flatMap is used "in certain circumstances", but in my experience it's more like 99% of all flatMaps on sequences are to deal with optionals, not to flatten nested sequences.

• Is the problem being addressed significant enough to warrant a change to Swift?

I don't think so. It's a fairly minor issue, one that really only affects new Swift programmers anyway rather than all users, and it will cause far too much code churn to be worthwhile.

I'd much rather see a proposal to add a new @available type, something like 'warning', that lets you attach an arbitrary warning message to a call (which you can kind of do with 'deprecated' except that makes the warning message claim the API is deprecated).

As review manager, I generally try to avoid commenting on threads, but I find this point interesting in a way that, if you don't mind, I'd like to explore.

Would this attribute not be a form of deprecation? Certainly it acts to discourage current and subsequent use, since every such use will evoke a warning.

Is the word "deprecation" just too strong? Often we think of deprecated APIs as being ones with more functional problems, like an inability to report errors, or semantics that must have seemed like a good idea at the time. Here it's just that the API has a name we don't like, and perhaps "deprecation" feels unnecessarily judgmental.

What I'm suggesting is that we don't change the API name at all. That's why I don't want to use 'deprecated', because we're not actually deprecating something. I'm just suggesting an alternative way of flagging cases where the user tries to use flatMap but accidentally invokes optional hoisting, and that's by making a new overload of flatMap that works for non-optional (non-sequence) values and warns the user that what they're doing is better done as a map. Using the 'deprecated' attribute for this would be confusing because it would make it sound like flatMap itself is deprecated when it's not.

I see. Thanks.

John.

Also, more practically, it conflates a relatively unimportant suggestion — that we should call the new method in order to make our code clearer — with a more serious one — that we should revise our code to stop using a problematic API. Yes, the rename has a fix-it, but still: to the extent that these things demand limited attention from the programmer, that attention should clearly be focused on the latter set of problems. Perhaps that sense of severity is something that an IDE should take into consideration when reporting problems.

What else would you have in mind for this warning?

The main use for this warning would be for adding overloads to methods that take optionals in order to catch the cases where people invoke optional hoisting, so we can tell them that there's a better way to handle it if they don't have an optional. flatMap vs map is the obvious example, but I'm sure there are other cases where we can do this too.

But there are also other once-off uses. For example, in the past I've written a function that should only ever be used for debugging, so I marked it as deprecated with a message saying 'remove this before committing your code'. This warning would have been better done using the new 'warning' attribute instead of as a deprecation notice.

-Kevin Ballard

John.

With that sort of thing we could then declare

extension Sequence {
    @available(*, warning: "Use map instead")
    func flatMap<U>(_ f: (Element) -> U) -> [U] {
        return map(f)
    }
}

And now if someone writes flatMap in a way that invokes optional hoisting, it'll match this overload instead and warn them.

• How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

A quick reading, and a couple of minutes testing overload behavior with availability attributes (to confirm that we can't simply use 'unavailable' for this).

-Kevin Ballard

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Ben_Cohen · November 9, 2017, 7:48pm

The problem in the Doodads example is that the name flatMap is used to identify two distinct intents: concatenating arrays and filtering nils. One can argue that those two operations are, in some lofty abstract sense, if you squint, two instances of some more general pattern — but I don’t think it’s fair to say that they represent the same intent. These separate intents deserve separate names.

They absolutely represent the same intent if you think of an optional as a collection of zero or one elements.

-Kevin Ballard

But as `Optional` does not conform to collection, I would suggest that the vast majority of of developers do _not_ think of them as a collection of zero or one elements. (I certainly don’t know any who naturally think of it that way.) We don’t treat Optional as a collection anywhere else in the API, and it seems odd to do so in just this one case.

And, to be clear, the lack of Collection conformance by Optional is an active choice, not an omission.

The “think of optionals as collections” explanation is a good way to help people who are confused by the overload. But an even better way would be to not have a confusing overload in the first place.

···

On Nov 9, 2017, at 10:45 AM, BJ Homer via swift-evolution <swift-evolution@swift.org> wrote:
On Nov 9, 2017, at 11:36 AM, Kevin Ballard via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Wed, Nov 8, 2017, at 09:29 PM, Paul Cantrell via swift-evolution wrote:

-BJ
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

masters3d · November 9, 2017, 9:45pm

- 1 I agree with Kevin Ballard.

I think that it would be appropriate to nudge the user to use map instead.

We already nudge people to use let over var so I think is sensible to do the same for misuses of flatMap when map is sufficient.

Thanks!

···

On Nov 8, 2017, at 10:43 AM, John McCall via swift-evolution <swift-evolution@swift.org> wrote:

On Nov 8, 2017, at 1:20 PM, Kevin Ballard <kevin@sb.org <mailto:kevin@sb.org>> wrote:

On Tue, Nov 7, 2017, at 09:37 PM, John McCall wrote:

On Nov 7, 2017, at 6:34 PM, Kevin Ballard via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Tue, Nov 7, 2017, at 03:23 PM, John McCall via swift-evolution wrote:

https://github.com/apple/swift-evolution/blob/master/proposals/0187-introduce-filtermap.md

• What is your evaluation of the proposal?

This proposal is going to cause an insane amount of code churn. The proposal suggests this overload of flatMap is used "in certain circumstances", but in my experience it's more like 99% of all flatMaps on sequences are to deal with optionals, not to flatten nested sequences.

• Is the problem being addressed significant enough to warrant a change to Swift?

I don't think so. It's a fairly minor issue, one that really only affects new Swift programmers anyway rather than all users, and it will cause far too much code churn to be worthwhile.

I'd much rather see a proposal to add a new @available type, something like 'warning', that lets you attach an arbitrary warning message to a call (which you can kind of do with 'deprecated' except that makes the warning message claim the API is deprecated).

As review manager, I generally try to avoid commenting on threads, but I find this point interesting in a way that, if you don't mind, I'd like to explore.

Would this attribute not be a form of deprecation? Certainly it acts to discourage current and subsequent use, since every such use will evoke a warning.

Is the word "deprecation" just too strong? Often we think of deprecated APIs as being ones with more functional problems, like an inability to report errors, or semantics that must have seemed like a good idea at the time. Here it's just that the API has a name we don't like, and perhaps "deprecation" feels unnecessarily judgmental.

What I'm suggesting is that we don't change the API name at all. That's why I don't want to use 'deprecated', because we're not actually deprecating something. I'm just suggesting an alternative way of flagging cases where the user tries to use flatMap but accidentally invokes optional hoisting, and that's by making a new overload of flatMap that works for non-optional (non-sequence) values and warns the user that what they're doing is better done as a map. Using the 'deprecated' attribute for this would be confusing because it would make it sound like flatMap itself is deprecated when it's not.

I see. Thanks.

John.

Also, more practically, it conflates a relatively unimportant suggestion — that we should call the new method in order to make our code clearer — with a more serious one — that we should revise our code to stop using a problematic API. Yes, the rename has a fix-it, but still: to the extent that these things demand limited attention from the programmer, that attention should clearly be focused on the latter set of problems. Perhaps that sense of severity is something that an IDE should take into consideration when reporting problems.

What else would you have in mind for this warning?

The main use for this warning would be for adding overloads to methods that take optionals in order to catch the cases where people invoke optional hoisting, so we can tell them that there's a better way to handle it if they don't have an optional. flatMap vs map is the obvious example, but I'm sure there are other cases where we can do this too.

But there are also other once-off uses. For example, in the past I've written a function that should only ever be used for debugging, so I marked it as deprecated with a message saying 'remove this before committing your code'. This warning would have been better done using the new 'warning' attribute instead of as a deprecation notice.

-Kevin Ballard

John.

With that sort of thing we could then declare

extension Sequence {
    @available(*, warning: "Use map instead")
    func flatMap<U>(_ f: (Element) -> U) -> [U] {
        return map(f)
    }
}

And now if someone writes flatMap in a way that invokes optional hoisting, it'll match this overload instead and warn them.

• How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

A quick reading, and a couple of minutes testing overload behavior with availability attributes (to confirm that we can't simply use 'unavailable' for this).

-Kevin Ballard

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Shawn_Erickson · November 13, 2017, 6:11pm

Terms like "ignoring" and "skipping".

···

On Mon, Nov 13, 2017 at 10:02 AM C. Keith Ray via swift-evolution < swift-evolution@swift.org> wrote:

On Nov 12, 2017, at 10:54 AM, Paul Cantrell via swift-evolution < > swift-evolution@swift.org> wrote:

On Nov 11, 2017, at 8:35 AM, Gwendal Roué via swift-evolution < > swift-evolution@swift.org> wrote:

I'd suggest `compactMap` as an alternative name, should `filterMap` find
too much resistance:

I like that name too, though it’s probably my Ruby experience showing. I
like the clarity of “compact” for nils, “filter” for booleans. I like how
fluently “compactMap” reads.

If the core team does accept this proposal, I’d trust them to take a
minute to make a considered, opinionated choice about the name.

You said "compact" removes nil values, so I'd name it "removeNils".

lancep · November 9, 2017, 10:59pm

The idea is that renaming this will nudge people into using map when appropriate.

···

On Nov 9, 2017, at 1:45 PM, Jose Cheyo Jimenez via swift-evolution <swift-evolution@swift.org> wrote:

- 1 I agree with Kevin Ballard.

I think that it would be appropriate to nudge the user to use map instead.

We already nudge people to use let over var so I think is sensible to do the same for misuses of flatMap when map is sufficient.

Thanks!

On Nov 8, 2017, at 10:43 AM, John McCall via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Nov 8, 2017, at 1:20 PM, Kevin Ballard <kevin@sb.org <mailto:kevin@sb.org>> wrote:

On Tue, Nov 7, 2017, at 09:37 PM, John McCall wrote:

On Nov 7, 2017, at 6:34 PM, Kevin Ballard via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Tue, Nov 7, 2017, at 03:23 PM, John McCall via swift-evolution wrote:

https://github.com/apple/swift-evolution/blob/master/proposals/0187-introduce-filtermap.md

• What is your evaluation of the proposal?

This proposal is going to cause an insane amount of code churn. The proposal suggests this overload of flatMap is used "in certain circumstances", but in my experience it's more like 99% of all flatMaps on sequences are to deal with optionals, not to flatten nested sequences.

• Is the problem being addressed significant enough to warrant a change to Swift?

I don't think so. It's a fairly minor issue, one that really only affects new Swift programmers anyway rather than all users, and it will cause far too much code churn to be worthwhile.

I'd much rather see a proposal to add a new @available type, something like 'warning', that lets you attach an arbitrary warning message to a call (which you can kind of do with 'deprecated' except that makes the warning message claim the API is deprecated).

As review manager, I generally try to avoid commenting on threads, but I find this point interesting in a way that, if you don't mind, I'd like to explore.

Would this attribute not be a form of deprecation? Certainly it acts to discourage current and subsequent use, since every such use will evoke a warning.

Is the word "deprecation" just too strong? Often we think of deprecated APIs as being ones with more functional problems, like an inability to report errors, or semantics that must have seemed like a good idea at the time. Here it's just that the API has a name we don't like, and perhaps "deprecation" feels unnecessarily judgmental.

What I'm suggesting is that we don't change the API name at all. That's why I don't want to use 'deprecated', because we're not actually deprecating something. I'm just suggesting an alternative way of flagging cases where the user tries to use flatMap but accidentally invokes optional hoisting, and that's by making a new overload of flatMap that works for non-optional (non-sequence) values and warns the user that what they're doing is better done as a map. Using the 'deprecated' attribute for this would be confusing because it would make it sound like flatMap itself is deprecated when it's not.

I see. Thanks.

John.

Also, more practically, it conflates a relatively unimportant suggestion — that we should call the new method in order to make our code clearer — with a more serious one — that we should revise our code to stop using a problematic API. Yes, the rename has a fix-it, but still: to the extent that these things demand limited attention from the programmer, that attention should clearly be focused on the latter set of problems. Perhaps that sense of severity is something that an IDE should take into consideration when reporting problems.

What else would you have in mind for this warning?

The main use for this warning would be for adding overloads to methods that take optionals in order to catch the cases where people invoke optional hoisting, so we can tell them that there's a better way to handle it if they don't have an optional. flatMap vs map is the obvious example, but I'm sure there are other cases where we can do this too.

But there are also other once-off uses. For example, in the past I've written a function that should only ever be used for debugging, so I marked it as deprecated with a message saying 'remove this before committing your code'. This warning would have been better done using the new 'warning' attribute instead of as a deprecation notice.

-Kevin Ballard

John.

With that sort of thing we could then declare

extension Sequence {
    @available(*, warning: "Use map instead")
    func flatMap<U>(_ f: (Element) -> U) -> [U] {
        return map(f)
    }
}

And now if someone writes flatMap in a way that invokes optional hoisting, it'll match this overload instead and warn them.

• How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

A quick reading, and a couple of minutes testing overload behavior with availability attributes (to confirm that we can't simply use 'unavailable' for this).

-Kevin Ballard

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org <mailto:swift-evolution@swift.org>
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Ben_Cohen · November 10, 2017, 12:09am

Not to mention, makes it do a lot more work, lengthening compile times (especially when mixed with closure and literal type inference… the combinations, they explode).

···

On Nov 9, 2017, at 4:01 PM, Paul Cantrell <paul@bustoutsolutions.com> wrote:

using the same name for both flatMap variants causes the type checker to miss programmer errors

Paul_Cantrell · November 10, 2017, 12:18am

The problem in the Doodads example is that the name flatMap is used to identify two distinct intents: concatenating arrays and filtering nils. One can argue that those two operations are, in some lofty abstract sense, if you squint, two instances of some more general pattern — but I don’t think it’s fair to say that they represent the same intent. These separate intents deserve separate names.

They absolutely represent the same intent if you think of an optional as a collection of zero or one elements.

This is precisely the “lofty abstract sense, if you squint” to which I was referring. I stand by that characterization. Just because there is a clever isomorphism between two types doesn’t mean people use them with equivalent intent.

Splitting that hair aside, the point of that example code stands: using the same name for both flatMap variants causes the type checker to miss programmer errors it would otherwise catch. (And unlike the first motivating example in the proposal, the “warn on optional hoisting” feature would not help at all.)

But as `Optional` does not conform to collection, I would suggest that the vast majority of of developers do _not_ think of them as a collection of zero or one elements. (I certainly don’t know any who naturally think of it that way.) We don’t treat Optional as a collection anywhere else in the API, and it seems odd to do so in just this one case.

And, to be clear, the lack of Collection conformance by Optional is an active choice, not an omission.

The “think of optionals as collections” explanation is a good way to help people who are confused by the overload. But an even better way would be to not have a confusing overload in the first place.

Exactly so. A much crisper way of stating my sprawling argument.

I’m personally grateful that Swift reminds me that, for example, I need the question mark in view.gestureRecognizers?.count. It would be maddening if view.gestureRecognizers.count compiled, and always returned either 0 or 1. Imagine it!

    view.gestureRecognizers.count // returns 0
    view.gestureRecognizers =
    view.gestureRecognizers.count // now returns 1 (wat?)
    view.gestureRecognizers = [foo, bar, baz]
    view.gestureRecognizers.count // still returns 1 (wat?!)

That is a recipe for torches and pitchforks right there. Yet it is analogous to what flatMap currently does.

Cheers,

Paul

···

On Nov 9, 2017, at 1:48 PM, Ben Cohen via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Nov 9, 2017, at 10:45 AM, BJ Homer via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
On Nov 9, 2017, at 11:36 AM, Kevin Ballard via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Wed, Nov 8, 2017, at 09:29 PM, Paul Cantrell via swift-evolution wrote:

Tino · November 10, 2017, 12:07pm

The “think of optionals as collections” explanation is a good way to help people who are confused by the overload. But an even better way would be to not have a confusing overload in the first place.

With the renaming, confusion might strike less often, but with bigger impact (free translation of a real discussion):

···

A: Cool, filterMap makes much more sense than flatMap!
Me: Why do you thing so?
A: Well, it does a mapping, and then applies a filter to get rid of nils, and that's super useful!
Me: I see. So you can be sure that when you apply the filterMap method, the result doesn’t contain any nil values?
A: Don’t you understand the concept of a filter? Of course, that’s what I just said, you never have to worry about nil!

[me shows a very simple proof that this is completely wrong]

Even here on the mailing lists, people seem to have a wrong understanding of what flatMap actually does, and this is reinforced with the renaming.
It might have no considerable effect on real-world code, because even with false assumptions, correct results are possible — but I really would prefer to leave the name filterMap free for something like this:

extension Collection {
  func filterMap<T>(transform: (Element) -> T, include: (T) -> Bool) -> [T] {
    return self.map(transform).filter(include)
  }
}

Brent_Royal-Gordon · November 14, 2017, 12:57am

Keep in mind that this does *slightly* more than just remove nils—it also removes a level of optionality so that it isn't *possible* for there to be nils anymore. It's the difference between `if let foo = foo` and `if foo != nil`.

···

On Nov 13, 2017, at 10:11 AM, Shawn Erickson via swift-evolution <swift-evolution@swift.org> wrote:

On Mon, Nov 13, 2017 at 10:02 AM C. Keith Ray via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

You said "compact" removes nil values, so I'd name it "removeNils".

Terms like "ignoring" and "skipping".

--
Brent Royal-Gordon
Architechies

Shawn_Erickson · November 14, 2017, 6:45am

Yeah but it seems clear from the return type so I am not sure that much
confusion would really exist.

···

On Mon, Nov 13, 2017 at 4:57 PM Brent Royal-Gordon <brent@architechies.com> wrote:

On Nov 13, 2017, at 10:11 AM, Shawn Erickson via swift-evolution < > swift-evolution@swift.org> wrote:

On Mon, Nov 13, 2017 at 10:02 AM C. Keith Ray via swift-evolution < > swift-evolution@swift.org> wrote:

You said "compact" removes nil values, so I'd name it "removeNils".

Terms like "ignoring" and "skipping".

Keep in mind that this does *slightly* more than just remove nils—it also
removes a level of optionality so that it isn't *possible* for there to be
nils anymore. It's the difference between `if let foo = foo` and `if foo !=
nil`.

--
Brent Royal-Gordon
Architechies

mark · November 10, 2017, 2:22am

+1

• What is your evaluation of the proposal?

I am highly in favor of this proposal. I have seen so many of my peers
struggle with custom types, like Result and Observable, that implement a
functional flatMap as opposed to the aforementioned flatMap that I've seen
create a barrier of learning. While my observation is anecdotal, it speaks
to an existing, greater churn to a journey of understanding as opposed to a
provisional swift migration.

• Is the problem being addressed significant enough to warrant a change to
Swift?

Yes.

• Does this proposal fit well with the feel and direction of Swift?

Yes.

• If you have used other languages or libraries with a similar feature, how
do you feel that this proposal compares to those?

This change will properly align the definition of flatMap across other
languages.

• How much effort did you put into your review? A glance, a quick reading,
or an in-depth study?

A quick reading.

···

On Thu, Nov 9, 2017 at 6:18 PM, Paul Cantrell via swift-evolution < swift-evolution@swift.org> wrote:

On Nov 9, 2017, at 1:48 PM, Ben Cohen via swift-evolution < > swift-evolution@swift.org> wrote:

On Nov 9, 2017, at 10:45 AM, BJ Homer via swift-evolution < > swift-evolution@swift.org> wrote:

On Nov 9, 2017, at 11:36 AM, Kevin Ballard via swift-evolution < > swift-evolution@swift.org> wrote:

On Wed, Nov 8, 2017, at 09:29 PM, Paul Cantrell via swift-evolution wrote:

The problem in the Doodads example is that *the name flatMap is used to
identify two distinct intents*: concatenating arrays and filtering nils.
One can argue that those two operations are, in some lofty abstract sense,
if you squint, two instances of some more general pattern — but I don’t
think it’s fair to say that they represent the same *intent*. These
separate intents deserve separate names.

They absolutely represent the same intent if you think of an optional as a
collection of zero or one elements.

This is precisely the “lofty abstract sense, if you squint” to which I was
referring. I stand by that characterization. Just because there is a clever
isomorphism between two types doesn’t mean people use them with equivalent
intent.

Splitting that hair aside, the point of that example code stands: using
the same name for both flatMap variants causes the type checker to miss
programmer errors it would otherwise catch. (And unlike the first
motivating example in the proposal, the “warn on optional hoisting” feature
would not help at all.)

But as `Optional` does not conform to collection, I would suggest that the
vast majority of of developers do _not_ think of them as a collection of
zero or one elements. (I certainly don’t know any who naturally think of it
that way.) We don’t treat Optional as a collection anywhere else in the
API, and it seems odd to do so in just this one case.

And, to be clear, the lack of Collection conformance by Optional is an
active choice, not an omission.

The “think of optionals as collections” explanation is a good way to help
people who are confused by the overload. But an even better way would be to
not have a confusing overload in the first place.

Exactly so. A much crisper way of stating my sprawling argument.

I’m personally grateful that Swift reminds me that, for example, I need
the question mark in view.gestureRecognizers?.count. It would be
maddening if view.gestureRecognizers.count compiled, and always returned
either 0 or 1. Imagine it!

    view.gestureRecognizers.count // returns 0
    view.gestureRecognizers =
    view.gestureRecognizers.count // now returns 1 (wat?)
    view.gestureRecognizers = [foo, bar, baz]
    view.gestureRecognizers.count // still returns 1 (wat?!)

That is a recipe for torches and pitchforks right there. Yet it is
analogous to what flatMap currently does.

Cheers,

Paul

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

bjhomer · November 10, 2017, 3:37pm

I prefer to think of it this way: it does a mapping, and keeps the results where the closure did not return nil. In the example given earlier, the closure returned Optional<Int?>.some(nil), which is not “nil”.

-BJ

···

On Nov 10, 2017, at 5:07 AM, Tino Heth <2th@gmx.de> wrote:

A: Well, it does a mapping, and then applies a filter to get rid of nils,

Ben_Cohen · November 10, 2017, 5:28pm

If you insist upon a literal translation of the name into code, then here:

extension Sequence {
    func filterMap<T>(_ transform: (Element)->T?) -> [T] {
        return self.filter { transform($0) != nil }.map { transform($0)! }
    }
}

The implementor is then able to optimize that implementation however they prefer.

···

On Nov 10, 2017, at 4:07 AM, Tino Heth <2th@gmx.de> wrote:

The “think of optionals as collections” explanation is a good way to help people who are confused by the overload. But an even better way would be to not have a confusing overload in the first place.

With the renaming, confusion might strike less often, but with bigger impact (free translation of a real discussion):
A: Cool, filterMap makes much more sense than flatMap!
Me: Why do you thing so?
A: Well, it does a mapping, and then applies a filter to get rid of nils, and that's super useful!
Me: I see. So you can be sure that when you apply the filterMap method, the result doesn’t contain any nil values?
A: Don’t you understand the concept of a filter? Of course, that’s what I just said, you never have to worry about nil!

[me shows a very simple proof that this is completely wrong]

Even here on the mailing lists, people seem to have a wrong understanding of what flatMap actually does, and this is reinforced with the renaming.
It might have no considerable effect on real-world code, because even with false assumptions, correct results are possible — but I really would prefer to leave the name filterMap free for something like this:

extension Collection {
  func filterMap<T>(transform: (Element) -> T, include: (T) -> Bool) -> [T] {
    return self.map(transform).filter(include)
  }
}

Tino · November 10, 2017, 11:31am

I’m personally grateful that Swift reminds me that, for example, I need the question mark in view.gestureRecognizers?.count. It would be maddening if view.gestureRecognizers.count compiled, and always returned either 0 or 1. Imagine it!
    view.gestureRecognizers.count // returns 0
    view.gestureRecognizers =
    view.gestureRecognizers.count // now returns 1 (wat?)
    view.gestureRecognizers = [foo, bar, baz]
    view.gestureRecognizers.count // still returns 1 (wat?!)

That is a recipe for torches and pitchforks right there. Yet it is analogous to what flatMap currently does.

[OT: NSView always returns an array here (which can be empty), and I think it is the right choice to avoid optional collections]

Even if the proposal is accepted, you still could do
v.gestureRecognizers.flatMap {
print($0)
}
as well as
v.gestureRecognizers?.flatMap {
print($0)
}

So we are not talking about fully abandoning the concept of a container with a flatMap-method, but rather break one part, and leave the other dangling around.

moiseev · November 10, 2017, 10:32pm

The “think of optionals as collections” explanation is a good way to help people who are confused by the overload. But an even better way would be to not have a confusing overload in the first place.

With the renaming, confusion might strike less often, but with bigger impact (free translation of a real discussion):
A: Cool, filterMap makes much more sense than flatMap!
Me: Why do you thing so?
A: Well, it does a mapping, and then applies a filter to get rid of nils, and that's super useful!
Me: I see. So you can be sure that when you apply the filterMap method, the result doesn’t contain any nil values?
A: Don’t you understand the concept of a filter? Of course, that’s what I just said, you never have to worry about nil!

[me shows a very simple proof that this is completely wrong]

Even here on the mailing lists, people seem to have a wrong understanding of what flatMap actually does, and this is reinforced with the renaming.
It might have no considerable effect on real-world code, because even with false assumptions, correct results are possible — but I really would prefer to leave the name filterMap free for something like this:

extension Collection {
  func filterMap<T>(transform: (Element) -> T, include: (T) -> Bool) -> [T] {
    return self.map(transform).filter(include)
  }
}

I understand the risk of diverging the discussion, but still, what’s the benefit of having this function over using the built-in `.lazy.map{}.filter{}` ?

···

On Nov 10, 2017, at 4:07 AM, Tino Heth via swift-evolution <swift-evolution@swift.org> wrote:

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Alejandro_Martinez · November 10, 2017, 4:35pm

github.com

apple/swift-evolution/blob/main/proposals/0187-introduce-filtermap.md

# Introduce Sequence.compactMap(_:)

* Proposal: [SE-0187](0187-introduce-filtermap.md)
* Author: [Max Moiseev](https://github.com/moiseev)
* Review Manager: [John McCall](https://github.com/rjmccall)
* Status: **Implemented (Swift 4.1)**
* Implementation: [apple/swift#12819](https://github.com/apple/swift/pull/12819)
* Decision Notes:
    [Review #0](https://forums.swift.org/t/draft-introduce-sequence-filteredmap/6872),
    [Review #1](https://forums.swift.org/t/review-se-0187-introduce-sequence-filtermap/6977),
    [Review #2](https://forums.swift.org/t/accepted-and-focused-re-review-se-0187-introduce-sequence-filtermap/7076),
    [Rationale](https://forums.swift.org/t/accepted-with-revisions-se-0187-introduce-sequence-filtermap/7290)
* Previous Revision: [1](https://github.com/apple/swift-evolution/blob/2d24b0ce9f138858b8341467170d6d8ba973827f/proposals/0187-introduce-filtermap.md)

## Introduction

We propose to deprecate the controversial version of a `Sequence.flatMap` method
and provide the same functionality under a different, and potentially more
descriptive, name.

This file has been truncated. show original

+1 to change it (but the filterMap doesn't' seem perfect neither)

• What is your evaluation of the proposal?

I'm in favour of renaming that specific variant of flatMap but I'm not
100% that filterMap is the best alternative name.

• Is the problem being addressed significant enough to warrant a
change to Swift?

Yes, maybe with a migration option to make the transition nicer

When introducing functional concepts the majority of people always
struggles with flatMap, and this specific Swift variant of it makes it
even harder. At the end I endup ignoring it for explanations and
telling them "and Swift has this specific one that is nice to have but
don't mind that it's called flatMap because it is not".
Is super convenient to have, so I still want that method to exist, but
the flatMap name invoques a specific known set of rules that this
version doesn't respect.

• Does this proposal fit well with the feel and direction of Swift?

Yes, having this convenience is really nice when writing code, but
making naming clear is also really important.

• If you have used other languages or libraries with a similar
feature, how do you feel that this proposal compares to those?

Not much to add here.

• How much effort did you put into your review? A glance, a quick
reading, or an in-depth study?

I wrote about this as soon as Swift was released years ago and have
been seeing the effect of this on every newcomer I've teached Swift.
But as I said, not a big fan the name suggested but I don't have
strong opinion about alternatives.

···

On Fri, Nov 10, 2017 at 3:37 PM, BJ Homer via swift-evolution <swift-evolution@swift.org> wrote:

On Nov 10, 2017, at 5:07 AM, Tino Heth <2th@gmx.de> wrote:

A: Well, it does a mapping, and then applies a filter to get rid of nils,

I prefer to think of it this way: it does a mapping, and keeps the results where the closure did not return nil. In the example given earlier, the closure returned Optional<Int?>.some(nil), which is not “nil”.

-BJ
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

--
Alejandro Martinez

Tino · November 10, 2017, 5:37pm

I prefer to think of it this way: it does a mapping, and keeps the results where the closure did not return nil. In the example given earlier, the closure returned Optional<Int?>.some(nil), which is not “nil”.

Exactly; therefore, flatMap can even „create“ nil values…
Filters in the physical world aren’t perfect either, and the spam problem has the same issues in the digital domain — but compared with that, eliminating nil is quite simple.
So people (not all, but imho many) will think that filterMap works like an idealized filter, where you pour dirty water in, and get a crystal clear and consumable result.

How would you rate a filter that adds dirt to its product, or a high-pass that adds a significant component below the cutoff frequency to a signal?

Filters aren’t supposed to do that, even if you feed them something strange like Optional<Optional<T>>… and when it adds something that wasn’t in the input, it might be a catalysator, but not a filter (the filter-method in Swift follows that rule as well: It can only map [T?] to [T?], not [T]).

So I have no doubt that flatMap is a better description of what is happening („you map, unwrap the results, and put them all in the same container“ — that’s true for collections as well as for optionals), but if many people have problems with it, something should be changed...
It wouldn’t be the first time where a problematic syntax is added because it pleases the user (whenever I write something like „array0 + array1“, the mathematician in me cries), but in this case, there is no need to choose something that veils what it is doing:
There are alternatives that don’t have this issue („collect“, „collectResults“...), but I actually would prefer just adding a warning to the problematic form of flatMap — simply because without that, nothing will stop people from misusing filterMap in the same way as they do with flatMap now.

VladimirS · November 10, 2017, 4:29pm

A: Well, it does a mapping, and then applies a filter to get rid of nils,

I prefer to think of it this way: it does a mapping, and keeps the results where the closure did not return nil. In the example given earlier, the closure returned Optional<Int?>.some(nil), which is not “nil”.

"So you can be sure that when you apply the filterMap method, the result doesn’t contain any nil values?"

Probably some code like this was shown then?:

let a : [[Int?]] = [[1,2,3,nil],[4,nil,5]]

print(a.flatMap{$0}) // [Optional(1), Optional(2), Optional(3), nil, Optional(4), nil, Optional(5)]

···

On 10.11.2017 18:37, BJ Homer via swift-evolution wrote:

On Nov 10, 2017, at 5:07 AM, Tino Heth <2th@gmx.de> wrote:

-BJ
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution