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

EitherOrBoth: Add or and or_else methods to simplify getting default values #593

Merged

Conversation

carl-anders
Copy link
Contributor

Introducing or and or_else methods to EitherOrBoth

This feature reduces the amount of code required to get specific default values when using EitherOrBoth.
An example of the current way of using zip_longest with custom default values:

let a = (0..=4).into_iter();
let b = (0..=2).into_iter();
let c = a.zip_longest(b).map(|e| match e {
    EitherOrBoth::Both(l, r) => (l, r),
    EitherOrBoth::Left(l) => (l, 2),
    EitherOrBoth::Right(r) => (4, r),
});
// c will now contain an iterator with (0,0), (1,1), (2,2), (3,2), (4,2).

An example with the proposed or method:

let a = (0..=4).into_iter();
let b = (0..=2).into_iter();
let c = a.zip_longest(b).map(|e| e.or(4, 2));
// c will now contain an iterator with (0,0), (1,1), (2,2), (3,2), (4,2).

I have also included the or_else method which does the same but with closures.

Contribute questions

Include tests for your new feature, preferably a QuickCheck test

There are no tests for the other EitherOrBoth methods, so I was unsure how to place them. However, I have added DocTest's to both methods. These examples allows for easier to understand documentation, and also tests the validity of the methods.

For new features, please first consider filing a PR to rust-lang/rust

The EitherOrBoth struct does not exist in rust std library.

Concerns

The naming is slightly inconsistent when compared to rust's std::Option, considering the naming from there the added methods should be named unwrap_or and unwrap_or_else.
However, this would then become inconsistent with the existing method EitherOrBoth::or_default. Which is why I went with the chosen names.
I can change the method names if needed, but then we should consider changing or_default too.

P.S.

The CHANGELOG.md file has the text Add EitherOrBoth::or_default (#583). This number is wrong, it should be #538.

@phimuemue
Copy link
Member

Thanks for this; a nice generalization of or_default! I think we can take this as it is.

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 29, 2021

Build succeeded:

@bors bors bot merged commit 6c4fc2f into rust-itertools:master Dec 29, 2021
4 checks passed
@jswrenn jswrenn added this to the next milestone Dec 29, 2021
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.

None yet

3 participants