[Review] SE-0032: Add find method to SequenceType

Chris_Lattner · April 28, 2016, 6:11pm

Hello Swift community,

The review of "SE-0032: Add find method to SequenceType" begins now and runs through May 3. The proposal is available here:

swift-evolution/0032-sequencetype-find.md at master · apple/swift-evolution · GitHub

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 the review manager.

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 you 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

swift-evolution/process.md at master · apple/swift-evolution · GitHub

Thank you,

-Chris Lattner
Review Manager

Patrick_Smith · April 28, 2016, 11:21pm

* What is your evaluation of the proposal?

Yes, seems like a great addition. I have wanted a method like this quite a few
times.

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

Yes, I think it adds an essential part to Sequence.

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

Yes, it’s surprising it wasn’t there already; it feels a very natural
addition.

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

.find() is the name I have seen elsewhere, and this proposed functionality is
the same.

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

A quick reading, and had followed some earlier discussion.

**Patrick Smith**

Hello Swift community,

The review of "SE-0032: Add find method to SequenceType" begins now and runs

through May 3. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0032

-sequencetype-find.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 the review

manager.

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 you 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

Thank you,

-Chris Lattner

Review Manager

···

On Apr 29 2016, at 4:11 am, Chris Lattner via swift-evolution <swift- evolution@swift.org> wrote:

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

masters3d · April 29, 2016, 4:43am

* What is your evaluation of the proposal?

I think it is too limiting and I’d rather have find() be merged with current index() method.

// I believe these used to be called find() in swift 1.0
func index(of element: Self.Iterator.Element) -> Self.Index?
func index(@noescape where predicate: (Self.Iterator.Element) throws -> Bool) rethrows -> Self.Index?

I think it is really confusing to have an index() and a find() that do very similar things.

I rather have an overloaded find()

func find(indexOf element: Self.Iterator.Element) -> Self.Index?
func find(@noescape indexOf predicate: (Self.Iterator.Element) throws -> Bool) rethrows -> Self.Index?
func find(@noescape element predicate: (Self.Iterator.Element) throws -> Bool) rethrows -> Self.Iterator.Element?

// Return a tuple with the index and element
func find(@noescape predicate: (Self.Iterator.Element) throws -> Bool) rethrows -> (index:Self.Index, element:Self.Iterator.Element)?

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

yes, I use filter for this but I believe find() and index() should be merged for it to be worth it.

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

yes but I believe index() should be renamed to find()

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

python.

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

read the proposal and suggested an extension of scope.

Erica_Sadun · April 29, 2016, 2:25pm

I like the functionality. I remain concerned that Sequence types do not guarantee termination and the semantics are better represented by something along the line of "ordered collection". Related notes here: kinds.md · GitHub

-- E

···

On Apr 28, 2016, at 12:11 PM, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0032: Add find method to SequenceType" begins now and runs through May 3. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0032-sequencetype-find.md

Rod_Brown · April 29, 2016, 5:31am

* What is your evaluation of the proposal?
+1 for the functionality on CollectionType
-1 for the functionality on SequenceType

* Is the problem being addressed significant enough to warrant a change to Swift?
It’s certainly a helpful addition and one I’ve got an analogous version of in my own projects. Considering this is a simply addition on CollectionType, I think there no reasons not to.

I’m concerned about this being added to SequenceType. The risks associated with iterating through a sequence where it may be destructive seem to great to me, especially when this can be limited to CollectionType instead.

* Does this proposal fit well with the feel and direction of Swift?
In general I find the proposal concept to be inline with Swift, for simplicity and ease of use. SequenceType usage however seems risky, due to the destructive iterator problem, and this seems against Swift’s safe direction.

* If you have you used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
I’ve used similar functionality added to NSArrays.

* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
A quick read, and was following the earlier discussion.

moiseev · April 30, 2016, 12:12am

HI all,

After having discussed this proposal with the members of the standard library team, we would like to propose the following updates:

- Since both the language and the standard library have evolved since it was written, the proposal should reflect these changes (things like renaming `Generator` to `Iterator`, adjusting for first argument label rules etc.)
(Actually, while writing this, I discovered https://github.com/apple/swift-evolution/pull/276\)
- We believe that renaming `find(_:)` to `first(where:)` would make call sites more clear

// original proposal
numbers.find { isPrime($0) }
numbers.find(isPrime)

// suggested update
numbers.first { isPrime($0) }
numbers.first(where: isPrime)

In the examples above, when the predicate is passed as a trailing closure, there is no big difference in the invocation, but it changes when we have a named function that we would like to use as a predicate.
The Collection protocol already has a property called `first`, that returns an optional element, it also has a method `index(where:)`. In this sense `first(where:)` does not introduce new words to the library vocabulary.

max

···

On Apr 28, 2016, at 11:11 AM, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0032: Add find method to SequenceType" begins now and runs through May 3. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0032-sequencetype-find.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 the review manager.

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 you 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

Thank you,

-Chris Lattner
Review Manager

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

dwaite · April 29, 2016, 2:48am

The review of "SE-0032: Add find method to SequenceType" begins now and runs through May 3. The proposal is available here:
https://github.com/apple/swift-evolution/blob/master/proposals/0032-sequencetype-find.md

* What is your evaluation of the proposal?

-0 for naming reasons

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

Sure

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

Not entirely. Because Sequence operations are based on the iterator, and the iterator is not guaranteed to be either non-destructive/resetting or finite, a find method could lead to subtle bugs.

I would prefer either another name such as skipUntil, or that find exist on Collection which does have the guarantee of being non-destructive and resettable on iteration.

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

skipUntil is the closest I see in reactive programming sources, which process events as a stream rather than as a resettable cursor into a data structure.

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

A quick reading.

Lily_Ballard · April 30, 2016, 12:36am

I don't understand this argument. The whole point of a sequence is to iterate over it. Yes, if you have a destructive sequence and you iterate over it multiple times it's not going to work right, but I don't see find() as having any higher risk of doing this than any of the other methods that iterate over the sequence.

Not only that, but I actually find the find() method to be most useful when used with Sequence. With Collection you can already just get the index and then use that to index into the collection; it's a little verbose, but not awful. But the only alternative with Sequence is to write an imperative loop that uses a mutable variable.

-Kevin

···

On Thu, Apr 28, 2016, at 10:31 PM, Rod Brown via swift-evolution wrote:

* What is your evaluation of the proposal?
+1 for the functionality on CollectionType
-1 for the functionality on SequenceType

* Is the problem being addressed significant enough to warrant a change to Swift?
It’s certainly a helpful addition and one I’ve got an analogous version of in my own projects. Considering this is a simply addition on CollectionType, I think there no reasons not to.

I’m concerned about this being added to SequenceType. The risks associated with iterating through a sequence where it may be destructive seem to great to me, especially when this can be limited to CollectionType instead.

Lily_Ballard · April 30, 2016, 12:42am

first(where:) is a neat idea, but I'm a little concerned about ambiguity
with the property in the presence of type errors. Experimentally, if I
try to call first(where:) with a block with the wrong signature, Swift 3
produces an unhelpful error about how I cannot call value of a non-
function type (i.e. the property) instead of recognizing that I'm trying
to call the function.

Example:

struct Foo {
var first: Int = 42

func first(where: @noescape Int -> Bool) -> Int {
return 42
}
}

If I try and call `print(Foo().find(where: { true }))`, you'll note that
the closure has the wrong type signature (it types as `() -> Bool`), and
the compiler gives me the error:

<REPL Input>:1:12: error: cannot call value of non-function type 'Int'
Foo().first(where: { true })
~~~~~~~~~~~^

Calling it with a block of the correct type works, but having useful
errors is very important.

-Kevin Ballard

···

On Fri, Apr 29, 2016, at 05:12 PM, Max Moiseev via swift-evolution wrote:

HI all,

After having discussed this proposal with the members of the standard
library team, we would like to propose the following updates:

- Since both the language and the standard library have evolved since
  it was written, the proposal should reflect these changes (things
  like renaming `Generator` to `Iterator`, adjusting for first
  argument label rules etc.)

(Actually, while writing this, I discovered
https://github.com/apple/swift-evolution/pull/276\)

- We believe that renaming `find(_:)` to `first(where:)` would make
  call sites more clear

// original proposal
numbers.find { isPrime($0) }
numbers.find(isPrime)

// suggested update
numbers.first { isPrime($0) }
numbers.first(where: isPrime)

In the examples above, when the predicate is passed as a trailing
closure, there is no big difference in the invocation, but it changes
when we have a named function that we would like to use as a
predicate.
The Collection protocol already has a property called `first`, that
returns an optional element, it also has a method `index(where:)`. In
this sense `first(where:)` does not introduce new words to the
library vocabulary.

masters3d · April 30, 2016, 12:45am

+1 for the name change.

···

On Apr 29, 2016, at 5:12 PM, Max Moiseev via swift-evolution <swift-evolution@swift.org> wrote:

HI all,

After having discussed this proposal with the members of the standard library team, we would like to propose the following updates:

- Since both the language and the standard library have evolved since it was written, the proposal should reflect these changes (things like renaming `Generator` to `Iterator`, adjusting for first argument label rules etc.)
(Actually, while writing this, I discovered https://github.com/apple/swift-evolution/pull/276\)
- We believe that renaming `find(_:)` to `first(where:)` would make call sites more clear

// original proposal
numbers.find { isPrime($0) }
numbers.find(isPrime)

// suggested update
numbers.first { isPrime($0) }
numbers.first(where: isPrime)

In the examples above, when the predicate is passed as a trailing closure, there is no big difference in the invocation, but it changes when we have a named function that we would like to use as a predicate.
The Collection protocol already has a property called `first`, that returns an optional element, it also has a method `index(where:)`. In this sense `first(where:)` does not introduce new words to the library vocabulary.

max

On Apr 28, 2016, at 11:11 AM, Chris Lattner via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Hello Swift community,

The review of "SE-0032: Add find method to SequenceType" begins now and runs through May 3. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0032-sequencetype-find.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 the review manager.

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 you 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

Thank you,

-Chris Lattner
Review Manager

_______________________________________________
swift-evolution mailing list
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

dabrahams · April 30, 2016, 12:51am

That's a QOI bug in the compiler that can be fixed. IMO we should avoid
designing APIs around temporary limitations of the compiler
implementation.

···

on Fri Apr 29 2016, Kevin Ballard <swift-evolution@swift.org> wrote:

On Fri, Apr 29, 2016, at 05:12 PM, Max Moiseev via swift-evolution wrote:

HI all,

After having discussed this proposal with the members of the standard
library team, we would like to propose the following updates:

- Since both the language and the standard library have evolved since
  it was written, the proposal should reflect these changes (things
  like renaming `Generator` to `Iterator`, adjusting for first
  argument label rules etc.)

(Actually, while writing this, I discovered
https://github.com/apple/swift-evolution/pull/276\)

- We believe that renaming `find(_:)` to `first(where:)` would make
  call sites more clear

// original proposal
numbers.find { isPrime($0) }
numbers.find(isPrime)

// suggested update
numbers.first { isPrime($0) }
numbers.first(where: isPrime)

In the examples above, when the predicate is passed as a trailing
closure, there is no big difference in the invocation, but it changes
when we have a named function that we would like to use as a
predicate.
The Collection protocol already has a property called `first`, that
returns an optional element, it also has a method `index(where:)`. In
this sense `first(where:)` does not introduce new words to the
library vocabulary.

first(where:) is a neat idea, but I'm a little concerned about ambiguity
with the property in the presence of type errors. Experimentally, if I
try to call first(where:) with a block with the wrong signature, Swift 3
produces an unhelpful error about how I cannot call value of a non-
function type (i.e. the property) instead of recognizing that I'm trying
to call the function.

Example:

struct Foo {
var first: Int = 42

func first(where: @noescape Int -> Bool) -> Int {
return 42
}
}

If I try and call `print(Foo().find(where: { true }))`, you'll note that
the closure has the wrong type signature (it types as `() -> Bool`), and
the compiler gives me the error:

<REPL Input>:1:12: error: cannot call value of non-function type 'Int'
Foo().first(where: { true })
~~~~~~~~~~~^

Calling it with a block of the correct type works, but having useful
errors is very important.

--
Dave