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

Hello, Swift community!

The review of "SE-0187: Introduce Sequence.filterMap(_:)" begins now and runs through November 14th, 2017. The proposal is available here:

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

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to me as the review manager. When replying, please try to keep the proposal link at the top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0187-introduce-filtermap.md
Reply text
Other replies
What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When writing your review, here are some questions you might want to answer in your review:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/master/process.md

As always, thank you for contributing to the evolution of Swift.

John McCall
Review Manager

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). 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

• What is your evaluation of the proposal?
I approve. This variant of ‘Sequence.flatMap’ is confusing to newcomers, and is inconsistent with the standard term-of-art usage of “flatMap”. People can learn what it means, but it continues to feel awkward. There will be some code churn, but it’s easily automatable and improves the clarity of the code. If we leave the existing spelling in place as deprecated, then we even preserve source compatibility.

• Is the problem being addressed significant enough to warrant a change to Swift?
I think so. The existing spelling of this functionality has three problems:
Discoverability: Users looking to filter out ‘nil’ values have to know to look for something that doesn’t say “filter”
Unexpected behavior: Users who are unfamiliar with the concept of Optionals as a Sequence-of-one have difficulty understanding why this is called “flatMap”. They don’t know what it does. I’ve seen this many times.
Brittleness: The functionality changes dramatically if the body of the function changes from returning an Optional to a Sequence. An example of this was given in the proposal, but here’s another one:
struct Thing {
    var name: String? // What if this becomes non-optional in the future?
}

// If Thing.name is optional, this returns an array of names (with nil filtered out)
// If Thing.name becomes non-optional, this now returns an array of all the characters in all the names, concatenated
things.flatMap { $0.name }

In general, changing from optional to non-optional causes compiler warnings in cases where something wasn’t expecting that. This isn’t likely a common problem, but it isn’t a great argument in defense of the current situation.

• Does this proposal fit well with the feel and direction of Swift?
Yes. Clear names are a common theme in Swift, and we have other proposals in Swift 5 with a similar goal.

• If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
It makes us behavior more like those other languages, reducing user surprise.

• How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
I was in the process of writing up a similar proposal myself when this one came out. I’ve thought about this quite a bit, and I think it’s the right choice.

-BJ

···

On Nov 7, 2017, at 4:23 PM, John McCall via swift-evolution <swift-evolution@swift.org> wrote:

Hello, Swift community!

The review of "SE-0187: Introduce Sequence.filterMap(_:)" begins now and runs through November 14th, 2017. The proposal is available here:

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

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to me as the review manager. When replying, please try to keep the proposal link at the top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0187-introduce-filtermap.md
Reply text
Other replies
What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When writing your review, here are some questions you might want to answer in your review:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/master/process.md

As always, thank you for contributing to the evolution of Swift.

John McCall
Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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.

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?

John.

···

On Nov 7, 2017, at 6:34 PM, Kevin Ballard via swift-evolution <swift-evolution@swift.org> wrote:
On Tue, Nov 7, 2017, at 03:23 PM, John McCall via swift-evolution wrote:

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
https://lists.swift.org/mailman/listinfo/swift-evolution

-1

I guess breaking existing code will be the show stopper for this proposal — but I generally think that compatibility is a poor rationale to stop an improvement, so my personal reasons are different:
The name is just wrong.
Just have a look at this simple example

extension Int {
    func justImagineError() throws -> Int {
        return self
    }
}

let ints: [Int?] = [nil]

let result = ints.flatMap {
    return try? $0?.justImagineError()
}
print(result)

If flatMap would really filter out nil values, this should yield an empty array as result — but the actual output is [nil]

+1

  • What is your evaluation of the proposal?

BJ summarized my thoughts nicely. I think “flatMap” in its current form is confusing to both newcomers and functional programmers familiar with the nomenclature. I’ve been aware of this disparity and thought about it for awhile. I typically redefine “flatMap” as “mapOptional” in my projects for clarity.

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

I think so! It improves the user experience in a significant way to those that encounter the function in the wild.

  • 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?

It fits in well. Swift’s current extra implementation of “flatMap” is the anomaly.

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

Gave a quick reading to the proposal.

···

On Nov 7, 2017, at 6:23 PM, John McCall <rjmccall@apple.com> 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’,

Please write one, seriously!

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). 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)
    }
}

FWIW: This was attempted in the past, and had to be reverted for the reason other than «deprecated» being a confusing message.
https://github.com/apple/swift/pull/9390

···

On Nov 7, 2017, at 3:34 PM, Kevin Ballard via swift-evolution <swift-evolution@swift.org> wrote:
On Tue, Nov 7, 2017, at 03:23 PM, John McCall via swift-evolution wrote:

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
https://lists.swift.org/mailman/listinfo/swift-evolution

I’ve seen enough misuse of flatMap at this point to be confident saying it’s not just a beginner error. Experienced programmers (as experienced as any Swift programmer can be for a 4-year-old language) frequently use flatMap when they mean map. It happens in Apple codebases, and in popular frameworks. Even with knowledgeable programmers, the explanation of the mistake they’ve made often takes a couple of minutes to sink in. The overload is actively confusing across the spectrum.

···

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

It's a fairly minor issue, one that really only affects new Swift programmers anyway rather than all users

When writing your review, here are some questions you might want to answer in your review:

  • What is your evaluation of the proposal?

-1. I am indifferent on the name, but don’t think it is worth the code churn.

What I would much rather see is an attribute that could be placed on parameters that would prevent optional promotion in those cases. That would prevent the issue listed here without needing to change the name, and it would be helpful in other places where optional promotion could cause mistakes.

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

No, I don’t really think so.

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

Meh. I think both the current name and the proposed name fit within the feel of swift.

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

There are a lot of languages with this functionality, they all have slightly different names for it. I think the name is fine, but if the name had been filterMap to start with that would have been ok too. I don’t think it is worth changing at this point though.

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

I followed the discussion, but didn’t spend a great deal of time thinking about it beyond that.

···

On Nov 7, 2017, at 3:23 PM, John McCall via swift-evolution <swift-evolution@swift.org> wrote:

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

  • What is your evaluation of the proposal?

-0. I think it would be more useful first to have an attribute that either warns or prevents optional promotion, so that misuse becomes a warning or error. If it remains a problem for readability, we can consider a rename.

However, I don’t like filterMap as a name, because unlike flatMap which is a combination of two things, filterMap is a combination of three - map, filter, and a hard-coded filter that rejects nil.

This seems just as likely to create odd misuse for people who know they want to map and filter, but who would be better served and have arguably cleaner code by having a lazy sequence rather than using mapFilter, e.g. (ignoring gender data often being an anti-pattern) a conversion from an array people to an array femaleFirstNames.

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

I think so, I’m just conflicted on whether this is the right solution.

  • 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?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

In-depth study

-DW

···

On Nov 7, 2017, at 4:23 PM, John McCall via swift-evolution <swift-evolution@swift.org> wrote:

The review of "SE-0187: Introduce Sequence.filterMap(_:)" begins now and
runs through November 14th, 2017. The proposal is available here:

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

• What is your evaluation of the proposal?
Worthwhile since there is a potential problem. However I am not convinced:

  1. About the name, it sounds like it filters and then maps rather than
what it really does which is map and then filter. Also, what does it
filter? How about mapFilterOptional (which is similar to Haskel name where
Maybe is equivalent of Optional).

  2. There is a more general problem of automatically wrapping a
non-optional in an optional; which this is a symptom off. So rather than
fix up the symptom, why not fix the problem. In particular; why not stop
the automatic conversion of a value to an optional, instead provide a value
to optional operator (the reverse of !). Say a trailing ~, used like 5~ to
mean Optional.some(5). ~ chosen because it is like an S, for some, on it's
side.

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

• Does this proposal fit well with the feel and direction of Swift?
Yes and no - see comments above. In particular automatic conversions are
not common in Swift. The conversion of a value to an optional automatically
therefore stands out.

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

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

  -- Howard.

···

On 8 November 2017 at 10:23, John McCall <rjmccall@apple.com> wrote:

Hello, Swift community!

The review of "SE-0187: Introduce Sequence.filterMap(_:)" begins now and
runs through November 14th, 2017. The proposal is available here:

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

Reviews are an important part of the Swift evolution process. All reviews
should be sent to the swift-evolution mailing list at

https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to me as the
review manager. When replying, please try to keep the proposal link at the
top of the message:

Proposal link:

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

Reply text

Other replies

*What goes into a review?*

The goal of the review process is to improve the proposal under review
through constructive criticism and, eventually, determine the direction of
Swift.

When writing your review, here are some questions you might want to answer
in your review:

• What is your evaluation of the proposal?
• Is the problem being addressed significant enough to warrant a change to
Swift?
• Does this proposal fit well with the feel and direction of Swift?
• If you have used other languages or libraries with a similar feature,
how do you feel that this proposal compares to those?
• How much effort did you put into your review? A glance, a quick reading,
or an in-depth study?

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/master/process.md

As always, thank you for contributing to the evolution of Swift.

John McCall
Review Manager

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

What is your evaluation of the proposal?

Strong approve.

I would happily also accept name variants such as mapFilter — but I’m heartily in favor of _something_ that distinguishes _by name_ flattening sequences vs. filtering nils.

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

The existing overloading of “flatMap” is not just confusing; it actively defeats some of the usefulness of Swift’s type checking. The proposal’s examples demonstrate this with the string example, but there’s another scenario (I think more compelling) that the proposal doesn’t mention. Consider this innocent-looking code:

    struct Doodad {
        public var words: [String]
    }

    // Meanwhile, elsewhere:

    let doodads = [
        Doodad(words: ["Some", "people", "like", "mustard"]),
        Doodad(words: ["even", "on", "custard"])
    ]

    let wordCount = doodads.flatMap { $0.words }.count

This counts 7 words, as expected. But if we make words optional, leaving the rest of the code unaltered:

        public var words: [String]?

…then suddenly wordCount changes to 2. Under the proposal, this would at least generate a deprecation warning.

Developing in Swift, I have the general expectation that if I make something optional, the type checker will help me discover the places where I need to deal with that change. Here it does not; instead, it silently changes the semantics of my existing code.

Note that the problem here has nothing to do with optional hoisting. If I understand correctly — and please do correct me if I’m wrong! — Kevin’s proposed alternative to flag unintended hoisting would not help solve this at all.

The problem here has instead to do with the fact that in programming languages, names serve as proxies for developer intent. That is, when we type, say, count, we are not just providing a human-readable GUID for a piece of code; we are implicitly referencing some mental model, some shared understanding of what that code we’re invoking is supposed to do. “Give me the count; we all know what that means, right?”

The fact that names capture developer intent is why static type checkers can help us reason about our code. Type systems catch developer errors because they can check a developer’s understanding of what they’re doing against the code’s own formal description of what it actually does (or at least some aspects thereof). If I write someOptionalVar.count, the compile error don’t just mean “that type doesn’t have that member;” it means something deeper and more useful: “You intend to count items, but that could be nil; how should I count nil? That code doesn’t mean what you think it means.”

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.

I don’t mean that we should take this to its absurd logical extreme and ensure that all method names are globally unique across the whole stdlib. But in this case, it doesn’t pay to intentionally create a name collision in a place where confusion is likely and the results mistakes are prone to going undetected. This overloading of flatMap has always bugged me, and I’d be pleased to see it cleaned up.

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

Clarity at the point of use and type safety are lynchpins of the language’s philosophy, yes.

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

Other languages I’ve used don’t conflate sequences and optionals the way Swift’s flatMap does. This proposal brings Swift into line with general convention.

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

A little light reading and too much writing.

Cheers, P

···

On Nov 7, 2017, at 5:23 PM, John McCall via swift-evolution <swift-evolution@swift.org> wrote:

Hello, Swift community!

The review of "SE-0187: Introduce Sequence.filterMap(_:)" begins now and runs through November 14th, 2017. The proposal is available here:

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

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

https://lists.swift.org/mailman/listinfo/swift-evolution
or, if you would like to keep your feedback private, directly to me as the review manager. When replying, please try to keep the proposal link at the top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0187-introduce-filtermap.md
Reply text
Other replies
What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When writing your review, here are some questions you might want to answer in your review:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/master/process.md

As always, thank you for contributing to the evolution of Swift.

John McCall
Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

• What is your evaluation of the proposal?

-1

Code churn, new function name has the same issues as the old one. Benefits small. It is also likely that those suffering with the String collectionifying will have already solved their issues before this update could be introduced.

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

No.

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

Distraction from major directions, little gain.

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

Read proposal and scanned the discussion so far in this review.

We did actually hit the bug where a String was flatMapped and behaviour changed in Swift 4 migration a couple of weeks ago which caused some discussion to find out what the issue was but we solved it and there is a fair chance the renamed version could equally have had exactly the same issue.

Joseph

• What is your evaluation of the proposal?

-1

Code churn, new function name has the same issues as the old one. Benefits small. It is also likely that those suffering with the String collectionifying will have already solved their issues before this update could be introduced.

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

No.

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

Distraction from major directions, little gain.

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

Read proposal and scanned the discussion so far in this review.

We did actually hit the bug where a String was flatMapped and behaviour changed in Swift 4 migration a couple of weeks ago which caused some discussion to find out what the issue was but we solved it and there is a fair chance the renamed version could equally have had exactly the same issue.

Joseph

  • What is your evaluation of the proposal?

One thing to keep in mind is that flatMap is basically the equivalent of optional chaining for non-`self` parameters.

  let result = character?.hexValue // optional chaining
  let result = character.flatMap { hexValue($0) } // optional "calling"?

I always found that `flatMap` was a weird name for this. Moreover, if the function you call is not returning an optional, `map` and `flatMap` essentially become synonyms. And thus I find myself using `flatMap` all the time when writing code like this, even when `map` would be enough, simply because it does what I expect all the time (like optional chaining, and unlike `map` that will instead pile up two layers of optionals).

The name `filterMap` makes a lot of sense for arrays, but it's still a bit awkward for optionals. I think the reason is that the most common thing you'd want to do with an optional is what `flatMap`/`filterMap` does, but the less specialized name (`map`) is already reserved with slightly different semantics. That is a bit dissonant in usage, even though the semantics makes sense in theory.

I think the most common use case, the one that mirrors optional chaining, should use the simplest name. I'm not sure what that word would be though.

[[
Side note: If only there was a nice way to express optional calling (when one of the parameters needs to be unwrapped or the call is skipped) like there is for optional chaining... I dream of this:

  let result = optional hexValue(character?)

where `optional` before an expression enables explicit unwrapping in subexpressions with `?` and any failure to unwrap a subexpression causes the optional expression to return `nil`. But that might be asking for too much sugar.
]]

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

I actually implemented `Sequence.filterMap` and `Optional.filterMap` in one of my projects and attempted to replace every `flatMap` with it. Looks like most of the `flatMap` calls in my project are used to filter optionals in arrays or doing optional calling. Only a tiny amount had to keep using `flatMap` because they were flattening nested arrays. That leads me to the conclusion that a better name would make most of the code more readable.

I think the change is worth it for arrays. Flattening an array is not the same thing as removing parts of it, so the word "filter" adds clarity.

I'm not sure it adds any clarity for optionals though.

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

I think it fits well for arrays. Not sure about optionals.

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

N/A

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

I implemented filterMap to see what it'd feel like.

···

--
Michel Fortin
https://michelf.ca

Hello,

  • What is your evaluation of the proposal?

I'm neutral. My humble contribution is about the name of the method, since many reactions so far have a problem with the proposed filterMap name.

I'm found of Ruby's `compact` method. Its role is to filter out nils:

  [1, 2, nil, 4].compact # => [1, 2 3]

This is not exactly what our discussed flatMap does, I know. Ruby's compact does not take a mapping closure. Swift's flatMap removes only one level of nils, as many contributors have reminded us. I know, I know.

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

  [1, 2, nil, 4].compactMap { $0 } // [1, 2, 4]
  ["1", "foo"]..compactMap { Int($0) } // [1]

I'd even suggest adding `compact()` as a shorthand for `compactMap { $0 }`, since filtering out nils as a very common operation.

"Compact" has the advantage of being a short word. "Compacting" a collection would just mean removing its nils, a concept that could easily stick in developers' minds, as Ruby has shown.

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

No. I too am guilty of having used flatMap when map was enough. Another name would not have changed this. It was just part of my learning phase.

Gwendal Roué

Hello, Swift community!

The review of "SE-0187: Introduce Sequence.filterMap(_:)" begins now and runs through November 14th, 2017. The proposal is available here:

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

  • What is your evaluation of the proposal?

It is a welcomed change provided that it does help with future integrity of the affected APIs.

But I am not sure that the solution is the best fit. It bends over the API while apparently the crux is the implicit optional promotion. Shouldn’t we consider the feasibility of disabling the promotion for this specific overload before proceeding with renaming? This wasn’t addressed in the proposal.

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

Yes. I agree that the unintended hole caused by future collection conformances and optional implicit promotion should be mitigated.

But in principle, I don’t see a problem with all of them being called `flatMap`, since `Optional` is effectively a 0- or 1-element collection, just without the `Collection` conformance.

  • 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?

ReactiveSwift calls this operation `filterMap` too (but operating on a reactive stream instead). As far as I could recall, we picked this name since (1) the operation is effectively map-then-filter; (2) it is named the same by Erlang, Elm, Rust, etc; and (3) we see no point to pull in new verbs to convey the meaning.

Some have expressed concerns about `filterMap` being the reverse of the actual operation (i.e. map-then-filter). While I have no exact idea of why `flatMap` was picked instead of `mapFlatten` by the early FP communities, my perception of the reasoning is that `mapFlatten(transform)` would be perceived as `mapFlattened` at first glance due to the LTR writing order.

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

A quick reading since these APIs are often used daily.

···

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

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/master/process.md

As always, thank you for contributing to the evolution of Swift.

John McCall
Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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.

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

···

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> wrote:>>

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
https://lists.swift.org/mailman/listinfo/swift-evolution

flatMap does filter out nil values. The problem is the return type of
your block is `Int??`, not `Int?`, so it's stripping off the outer layer
of optionals. Your example here is akin to one where your block returns
`[[[Int]]]` and asking why your resulting type is `[[Int]]`.
-Kevin Ballard

···

On Tue, Nov 7, 2017, at 05:23 PM, Tino Heth via swift-evolution wrote:

-1

I guess breaking existing code will be the show stopper for this
proposal — but I generally think that compatibility is a poor
rationale to stop an improvement, so my personal reasons are
different:> The name is just wrong.
Just have a look at this simple example

extension Int {
    func justImagineError() throws -> Int {
        return self
    }
}

let ints: [Int?] = [nil]

let result = ints.flatMap {
    return try? $0?.justImagineError()
}
print(result)

If flatMap would really filter out nil values, this should yield an
empty array as result — but the actual output is *[nil]*

Would this attribute not be a form of deprecation?

So far, I only encountered deprecation in the context of legacy functionality that will be removed later — but after taking a look in the dictionary, I think the word is actually a good fit (even if there are no plans to remove flatMap).

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.

I think there are worse warnings now:
Xcode suggests that a freshly declared var might be problematic, because you just did not have the time to write the code that changes it, that a let might be problematic because you can’t write the code to use that constant at the same time… I think this one here is rather helpful, as it educates you about a real (albeit small) error that can easily be avoided.

Even if filterMap is accepted, imho such a warning would be needed:
How should the same people that use flatMap wrong magically learn not to use filterMap all the time?