Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor Iterator::filter_map description rewording. #43965

Merged
merged 1 commit into from
Aug 20, 2017

Conversation

frewsxcv
Copy link
Member

Fixes #39294.

@rust-highfive
Copy link
Collaborator

r? @BurntSushi

(rust_highfive has picked a reviewer for you, use r? to override)

@frewsxcv
Copy link
Member Author

r? @QuietMisdreavus

@QuietMisdreavus
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 18, 2017

📌 Commit e29c020 has been approved by QuietMisdreavus

@frewsxcv
Copy link
Member Author

frewsxcv commented Aug 18, 2017

@bors r-

i thought about this again. consider this code:

["1", "foo", "3"]
    .iter()
    .filter_map(|n| n.parse::<i32>())
    .collect()

here's how you'd implement it with filter then map:

["1", "foo", "3"]
    .iter()
    .filter(|n| n.parse::<i32>().is_some())
    .map(|n| n.parse::<i32>().unwrap())
    .collect()

i can't think of a way to implement it with map then filter:

["1", "foo", "3"]
    .iter()
    .map(|n| n.parse::<i32>())
    .filter(|n| n.is_some())
    .collect()  // [Some(1), Some(3)]

so i think you're idea to do "filter and map" might be a better suggestion here. what do you think?

@QuietMisdreavus
Copy link
Member

The linked issue says it in terms of map(your_closure_here).filter(is_some).map(unwrap), so by that formulation it's technically map.filter.map. By saying "filter and map" you somewhat lose the ordering implied in the docs as-is, but even your own "map with filter" is better than what's there right now. I'm fine with either suggestion (though i am somewhat partial to the rhythm of "filter and map" >_>).

@frewsxcv
Copy link
Member Author

agreed with everything you said;switched to "map with filter"

@bors r=QuietMisdreavus

@bors
Copy link
Contributor

bors commented Aug 19, 2017

📌 Commit aac3008 has been approved by QuietMisdreavus

@bors
Copy link
Contributor

bors commented Aug 19, 2017

⌛ Testing commit aac3008 with merge 77213e740777020ee48e0d5f40ac033df2e908bb...

@bors
Copy link
Contributor

bors commented Aug 19, 2017

💔 Test failed - status-appveyor

@kennytm
Copy link
Member

kennytm commented Aug 19, 2017

@bors retry #43985

@bors
Copy link
Contributor

bors commented Aug 20, 2017

⌛ Testing commit aac3008 with merge 498a8f3...

bors added a commit that referenced this pull request Aug 20, 2017
Minor Iterator::filter_map description rewording.

Fixes #39294.
@bors
Copy link
Contributor

bors commented Aug 20, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: QuietMisdreavus
Pushing 498a8f3 to master...

@bors bors merged commit aac3008 into rust-lang:master Aug 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants