Add Iterator::find_or_{first,nth,last}

[z33ky]

I'm proposing to add the methods find_or_{first,nth,last} to the Iterator trait, to facilitate returning an item that is part of the sequence itself in case no item satisfies the predicate.

Example:

let a = [0, 1, 2, 3];

assert_eq!(a.iter().find_or_first(|&&e| e == 5   ), Some(&0)); // predicate is never satisfied, so `find_or_first` returns the first item (`&0`)
assert_eq!(a.iter().find_or_nth(  |&&e| e == 5, 1), Some(&1)); // predicate is never satisfied, so `find_or_nth` returns the 2nd item (`&1`) (count begins from `0`)
assert_eq!(a.iter().find_or_last( |&&e| e == 5   ), Some(&3)); // predicate is never satisfied, so `find_or_last` returns the last item (`&3`)

Some APIs return different formats or profiles/configurations that it supports and the developer shall choose one from them. Usually these have differing properties, where the developer can choose one according to their requirements or preferences. Examples for such an API can be found in Vulkan, like choosing a color-format where a listing obtained is via vkGetPhysicalDeviceSurfaceFormatsKHR (Surface::get_physical_device_surface_formats in ash, a wrapper library for Rust).

With such a list at hand, Iterator::find can be used to find a fitting option, and if none is found, any item should be selected instead. By providing additional methods in the Iterator trait, this task can be implemented elegantly.

For the mentioned use case the sequences that are processed may either have a specific order of preferences, with the most preferred one either first or last, or no communicated preference in the items.
In the former case (preference present), one would normally either want the first or last items as default, and in the latter it wouldn't matter. Either way, find_or_nth seems to not solve a concrete use case.
I added it for some semblance of "symmetry" with other item-retrieval methods already available on Iterator. Likewise, find_or_min and find_or_max could be added as well, though conceptually these would be like "find items with this (caller-provided) predicate, or else this other (hard-coded) predicate", which is a little weird, while the methods I intend to add are more like "find items with this predicate, or else return the item on this position", which in my head makes more sense to provide.
Maybe only find_or_first and find_or_last are really useful to have though and find_or_nth shouldn't be added to libstd.

I started implementing DoubleEndedIterator::rfind_or_{first,nth_back,last}, but again just for some semblance of symmetry. I think I'll see to some feedback first before spending time on those. An argument could perhaps be made for adding find_or_nth_back and rfind_or_nth in that case, too.

[rust-highfive]

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

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[ghost]

Suggested change

	/// leads to a possibly confusing situation where the argument is a double reference. You can
	/// leads to a special situation where the argument is a double reference. You can

Recommendation to remove passive wording.

[z33ky]

I copied the wording from Iterator::find. Shall I change it there as well? It also present in some other functions, e.g. Iterator::filter.

[ghost]

It's simply a matter of style. I realize that wording which suggests probable 'confusion' is commonly used throughout Rust code and documentation.

FWIW I'm not in any position to be making requests of contributors :)

[Dylan-DPC-zz]

r? @m-ou-se

[m-ou-se]

Thanks for your PR! And sorry for the delay.

Do you have some more motivating examples for adding these?

In the case of a Vec or array, .find(..).or(v.first()) (or last / get) would do the same, and adding new functions for this are probably not worth it. Your implementation does however clearly show that it isn't so simple for Iterators in general without having to iterate the iterator twice, which might be a good reason to add them.

Either way, find_or_nth seems to not solve a concrete use case.

Then it's probably better to not add that one for now. Just like find_or_max etc., it's probably a bit too specific with very few use cases.

[z33ky]

Thanks for your PR! And sorry for the delay.

I understand Rust is a busy project. Have no worries.

Do you have some more motivating examples for adding these?

The format listing mentioned in the first PR message is the only concrete example I stumbled upon, though I haven't attempted to look for further instances of this problem. I'm not sure how I would sanely approach making such a survey.
I can come up with some hypothetical examples, though I would think that only actual, concrete examples would strengthen the motivation.

In the case of a Vec or array, .find(..).or(v.first()) (or last / get) would do the same

This only works for .iter(), not .into_iter(), which means this only works if merely obtaining a borrowed item (Option<&mut _>) is fine. If you want to move the item out of a Vec, this approach will not work.

Then it's probably better to not add that one for now. Just like find_or_max etc., it's probably a bit too specific with very few use cases.

If find_or_{first,last}() are still desirable additions I will amend the PR to remove find_or_nth().
I suppose there is currently also no desire to add DoubleEndedIterator::rfind_or_{first,last}(). I will forego their implementation then; They can always be added later, if there is interest in them in the future.

[JohnCSimon]

Ping from triage - @z33ky
can you please post the status of this PR, thank you.

[z33ky]

@JohnCSimon you got it. There are three open questions.

1) Usefulness of this change

As @m-ou-se said:

In the case of a Vec or array, .find(..).or(v.first()) (or last / get) would do the same, and adding new functions for this are probably not worth it. Your implementation does however clearly show that it isn't so simple for Iterators in general without having to iterate the iterator twice, which might be a good reason to add them.

I'm stating that on top of these methods working on Iterators in general, the mentoned .find(..).or(v.first()) only works for getting a reference (or Copy) out of a Vec.
If you wanted to extract an item via move, you'd need v.into_iter().find(..).or(v.remove(0)), though the into_iter already moves v, so that v.remove(0) won't work in the same expression. Hence it'd actually need to become { let first = v.remove(0); v.into_iter().find(..).or(first) }.
remove() also needs to shift the elements, which is suboptimal (unless maybe if optimization can elide that). v.into_iter().find_or_first(..) is implemented such that the shift is not required. I also find it easier to read (and write).

2) rfind_or_{first,last}() for DoubleEndedIterator

Would it make sense to have rfind-variants on DoubleEndedIterator (rfind_or_{first,last}()) to complement the addition of the find_or_*-methods?

3) Wording on documentation

@kw-fn left a note about the wording of the documentation for the functions, which states "... leads to a possibly confusing situation where ...". They suggest using "special" in place of "possibly confusing".
I have copied the wording from Iterator::find() and also found it in the documentation for some other functions, like Iterator::filter().
I have no preference either way, so if any reviewers concur I have no issue changing the comment, and if desired also adapt the documentation on existing items.

---

The change-set of this PR currently also includes an Iterator::find_or_nth(). I already stated my doubt about its usefulness (as opposed to find_or_first() and find_or_last()) and @m-ou-se acknowledged my doubt. It should be removed from the PR. To me the implementations for the other two functions look fine though. If DoubleNededIterator::rfind_or_{first,last}()` shall be added I'm up for the task of implementing them as well.

[z33ky]

I should maybe also state that @m-ou-se asked me if I have more motivating examples. My reply was that I have not and I am unsure how I would approach making a survey for concrete examples.
I do not know how important it is to supply further motivation for this change. The question did not state its importance to the decision of merging this.

[bors]

☔ The latest upstream changes (presumably #76391) made this pull request unmergeable. Please resolve the merge conflicts.

[Dylan-DPC-zz]

@z33ky can you resolve the conflicts? thanks

[m-ou-se]

@rust-lang/libs What do y'all think about adding these Iterator methods?

[BurntSushi]

These methods look incredibly niche to me. They might be a better fit for the itertools crate.

I should maybe also state that @m-ou-se asked me if I have more motivating examples. My reply was that I have not and I am unsure how I would approach making a survey for concrete examples.
I do not know how important it is to supply further motivation for this change. The question did not state its importance to the decision of merging this.

Speaking for myself personally, concrete use cases are perhaps the most important thing to consider when adding new APIs.

[dtolnay]

Agreed -- I would prefer not to build these into Iterator.

[z33ky]

@Dylan-DPC

can you resolve the conflicts? thanks

I'd be up for it, but it looks like I'll be trying for itertools instead.

@BurntSushi

Speaking for myself personally, concrete use cases are perhaps the most important thing to consider when adding new APIs.

Yeah, makes sense. What good is code, even well written, if it goes unused.
I do have a generic description of the type of problem this can assist with (in the first post), but this only helps in coming up with imaginary scenarios.
As I stated before I'm not sure how I would make a survey of real-world usecases. Finding compatible formats for Vulkan is really the only one I stumbled upon. I haven't written a lot of Rust, but of course it's not the libs team's job to find more for me. If you remain unconvinced there's no point in dwelling on it, I have little stake on this.

[crlf0710]

Triage: I'm gonna close this according to T-libs members' response above. Feel free to raise questions in T-libs stream on official Zulip instance if you want to continue this discussion. Thanks for your work anyway!