Add Sequence.firstNonNil(_:)#31
Conversation
This feature felt minimal and self-explanatory enough to not be worth a guides doc, but I'll add one! |
| @@ -0,0 +1,45 @@ | |||
| # First | |||
There was a problem hiding this comment.
Since this API isn't first, but firstNonNil, can we align the naming of documents and headings accordingly?
There was a problem hiding this comment.
My thinking that it was a general category of functions for which others might be added. I'll amend it.
xwu
left a comment
xwu
left a comment
There was a problem hiding this comment.
It looks like we’ve settled on a name of firstNonNil(_:). Can you update the pull request title and message? They currently say that we’re adding an API called firstNonNil(of:).
|
|
||
|
|
||
| This method is analogous to `first(where:)` in how it only consumes values until | ||
| a `.some` is found, unlike using lazy operators, which will load any sequence into a collection |
There was a problem hiding this comment.
Could you be a little more explicit here about why this method is useful? I think @timvermeulen explained it very well in the corresponding issue (about using the alternative being unintentionally not lazy in certain situations), and if you get his permission, that might be a helpful piece of information for users.
| ### Comparison with other languages | ||
|
|
||
| **Scala**: Scala provides a `collectFirst` function that finds the first element | ||
| in a collection for which a partial function is defined. |
There was a problem hiding this comment.
There were a few other precedents mentioned in the corresponding issue. Could you please include them here? Ideally, we’d look around a bit more and really study the landscape here so that we can be sure we’ve picked a good name.
|
|
||
| ### Naming | ||
|
|
||
| This method’s name was selected for its comprehensibility. |
There was a problem hiding this comment.
I don’t think this sentence adds much to the reader. If this section isn’t mandatory, then it can be deleted. If it is mandatory, then you’ll need to explain why this name has good “comprehensibility.” Perhaps @natecook1000 can help you to explain since he made the choice.
There was a problem hiding this comment.
It would be great to capture the other names that came up in the issue discussion here. Why was firstNonNil chosen over those?
Co-authored-by: Xiaodi Wu <13952+xwu@users.noreply.github.com>
Co-authored-by: Xiaodi Wu <13952+xwu@users.noreply.github.com>
|
|
||
| ### Naming | ||
|
|
||
| This method’s name was selected for its comprehensibility. |
There was a problem hiding this comment.
It would be great to capture the other names that came up in the issue discussion here. Why was firstNonNil chosen over those?
| //===----------------------------------------------------------------------===// | ||
|
|
||
| public extension Sequence { | ||
| /// Returns the first element in `self` that `transform` maps to a `.some`. |
There was a problem hiding this comment.
This method doesn't return the first element, it returns the mapped result. Can you update this wording and include an example demonstrating the usage?
| for value in self { | ||
| if let value = try transform(value) { | ||
| return value | ||
| } | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Nit: For source files we use 2-space indents.
|
Hey @Qata, this is to let you know we'll soon be landing this addition and making the minor changes necessary in order to ship this in the next release. 🚀 Thank you for your work on this, and feel free to make any changes you still want to make in the meantime. |
|
@swift-ci please test |
5 participants
Resolves #21
Description
firstNonNil(_:)returns the first value that is resolved by the passed transform to a nonnil(.some) value.Detailed Design
Documentation Plan
Added documentation comments to the new function.
Test Plan
Five tests have been added.
Source Impact
None
Checklist