search

rust-itertools / itertools

Extra iterator adaptors, iterator methods, free functions, and macros.
https://docs.rs/itertools/
Apache License 2.0
2.72k stars 309 forks source link

add `{left,right}_or_{right,left}` methods to `EitherOrBoth` #721

Closed Testare closed 1 year ago

Testare commented 1 year ago

Been using this library for a bit, wanted to create some methods to cover a use case I kept coming across: I was zipping two iterators of the same type (zip_longest), taking the value from one if it was there but wanting to default to the other if not. Thought it would be a good contribution upstream! Did my homework to make sure it wasn't a duplicate of another PR, though saw other good PR's in this space.

I considered names like either_favor_right/either_favor_left, but left_or_right/right_or_left seemed the most ergonomic IMHO. It's a simple enough change if you disagree though.

phimuemue commented 1 year ago

If we take this, we should implement it as self.reduce(|l, _| l) resp. self.reduce(|_, r| r).

Looking at it, I am unsure if we really need this functionality. I see it's a bit more work to write it via reduce, but is it much more complex than a bespoke function? @Testare @jswrenn Do you have thoughts?

Testare commented 1 year ago

I'm fine with implementing the method that way. Reduce might cover this, though I didn't realize until a week or two before this PR that reduce was the method I was looking for. I felt like these names might better complement the methods "left", "right", and "both".

Perhaps it would be good to call out in the doc string for these methods that if you want different behavior for the both case, to look at "reduce." But if you feel this isn't a good fit for the package, I'd understand =]

phimuemue commented 1 year ago

I updated the docs for EitherOrBoth::reduce in https://github.com/rust-itertools/itertools/commit/052423a8c971e1ef46890840cd662ab7ddb2f809.