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

Add product_ok and sum_ok #814

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 53 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,8 @@ extern crate alloc;

#[cfg(feature = "use_alloc")]
use alloc::{string::String, vec::Vec};

pub use either::Either;

use core::borrow::Borrow;
pub use either::Either;
use std::cmp::Ordering;
#[cfg(feature = "use_std")]
use std::collections::HashMap;
Expand Down Expand Up @@ -144,8 +142,7 @@ pub mod traits {

pub use crate::concat_impl::concat;
pub use crate::cons_tuples_impl::cons_tuples;
pub use crate::diff::diff_with;
pub use crate::diff::Diff;
pub use crate::diff::{diff_with, Diff};
#[cfg(feature = "use_alloc")]
pub use crate::kmerge_impl::kmerge_by;
pub use crate::minmax::MinMaxResult;
Expand Down Expand Up @@ -2195,7 +2192,7 @@ pub trait Itertools: Iterator {
self.collect()
}

/// `.try_collect()` is more convenient way of writing
/// `.try_collect()` is a more convenient way of writing
/// `.collect::<Result<_, _>>()`
///
/// # Example
Expand Down Expand Up @@ -2223,6 +2220,56 @@ pub trait Itertools: Iterator {
self.collect()
}

/// `.product_ok()` is a more convenient way of writing `.product::<Result<_, _>>()`
///
/// **Panics** when a primitive integer type is returned and the computation
/// overflows, and debug assertions are enabled.
///
/// # Example
///
/// ```
/// use itertools::Itertools;
/// use std::str::FromStr;
///
/// fn main() -> Result<(), std::num::ParseIntError> {
/// let product: u64 = ["1", "2", "3"].iter().map(|x| u64::from_str(x)).product_ok()?;
/// assert_eq!(product, 6);
/// Ok(())
/// }
/// ```
fn product_ok<T, U, E>(self) -> Result<U, E>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this is an inelegantly large number of generic arguments.

This is a fairly radical alternative, but: What if we just mirrored the stdlib Try trait? We can pub-in-priv our Try-fork so that folks don't end up explicitly depending on it, and then simply (and potentially non-breaking-ly!) switch over to the standard library Try once it's stabilized.

If we do this, our *_ok functions will support more than just Results, and they'll be able to take just one generic argument, rather than three (I think).

Thoughts, @Philippe-Cholet and @phimuemue?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sure is radical but interesting, maybe it should have its own issue.
About the names, *_ok now and try_* later? (or maybe_*?!)

After a quick look, ControlFlow requires 1.55+ while we currently have 1.43. Unless we copy it too?
I previously read something about #[track_caller] but don't remember much about its use.

In libcore, fold is usually specialized using try_fold specializations alongside with NeverShortCircuit which surely won't be public forever so I thought that we would copy it at some point.

where
Self: Sized + Iterator<Item = Result<T, E>>,
Result<U, E>: std::iter::Product<Result<T, E>>,
{
self.product()
}

/// `.sum_ok()` is a more convenient way of writing `.sum::<Result<_, _>>()`
///
/// **Panics** when a primitive integer type is returned and the computation
/// overflows, and debug assertions are enabled.
///
/// # Example
///
/// ```
/// use itertools::Itertools;
/// use std::str::FromStr;
///
/// fn main() -> Result<(), std::num::ParseIntError> {
/// let sum: u64 = ["1", "2", "3"].iter().map(|x| u64::from_str(x)).sum_ok()?;
/// assert_eq!(sum, 6);
/// Ok(())
/// }
/// ```
fn sum_ok<T, U, E>(self) -> Result<U, E>
where
Self: Sized + Iterator<Item = Result<T, E>>,
Result<U, E>: std::iter::Sum<Result<T, E>>,
{
self.sum()
}

/// Assign to each reference in `self` from the `from` iterator,
/// stopping at the shortest of the two iterators.
///
Expand Down