[docs] In the documentation of the identify function, clarify that using flatten() is probably a better idea than using filter_map

[mousetail]

There is a odd example on the documentation for identity that seems to suggest using filter_map to flatten a array of Option<T>. However, this can be done far shorter using flatten.

The example is good to showcase how identity works but I think a clarification would be good so people don't actually use it that way.

I hope this extra note will help people decide for themselves what the best solution to this problem is.

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @m-ou-se (or someone else) soon.

Please see the contribution instructions for more information.

[cuviper]

Option is not an Iterator, but it does implement IntoIterator, which is sufficient for flatten().

Although flatten() is shorter to write, I personally still prefer filter_map(identity) because it is a much simpler iterator, and simple is usually better for optimization.

[mousetail]

I hadn't really thought about the optimization aspect, just that I prefer not to import more libraries. That makes the advice previously given in the docs not as bad as I thought.

[scottmcm]

Yeah, unfortunately .flatten() is a flat_map, not a filter_map, and thus fundamentally more complex. FlatMap is bigger than FilterMap because it needs to store the prefix and suffix iterators, as those might have more elements in them, whereas because FilterMap only deals with Options so knows they can't have another item in them after getting the one value out. (You can also see this by how, for example, filter_map preserves the upper bound on a size_hint, which flat_map doesn't, though admittedly the upper bounds on size hints are useless.) And this is why https://rust-lang.github.io/rust-clippy/master/index.html#flat_map_option exists.

I see that flat_map mentions flatten in the prose, but doesn't have an example -- maybe it'd make sense to move an example like this (perhaps with arrays instead of options) over to that method?

[scottmcm]

(Also, personally I'd never use identity here, since it needs a use and even ignoring that it isn't even shorter than writing out the |x| x closure. But since it's in the docs for identity and not for filter_map that seems fine.)

[mousetail]

That makes sense. Thank you for your insight, I especially appreciate including references and verifiable facts. I'll try to improve the documentation somewhere else.

I'll close this pull request.

[scottmcm]

Also, I'm glad you raised this, since it made me look for a performance lint in clippy talking about this, but instead I found exactly the opposite, a lint suggesting the .filter_map(identity) -> .flatten() change.

So I've filed rust-lang/rust-clippy#9377 for the clippy folks to discuss the situation.