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

impl std::iter::{Sum, Product} for Ratio #25

Merged
merged 2 commits into from
Apr 12, 2018

Conversation

Emerentius
Copy link
Contributor

Fixes #3

I only implemented Sum and Product for Ratio<T> and &Ratio<T> rather than any U, where Ratio<T>: Mul<U, Output = Ratio<T>>. Summing for example an iter of T directly into a Ratio<T> does not seem like particularly useful or critical functionality.

@cuviper
Copy link
Member

cuviper commented Apr 9, 2018

Thanks! How about some quick test cases?

Summing for example an iter of T directly into a Ratio does not seem like particularly useful or critical functionality.

I agree. Sums and products of integers can be done directly first.

@Emerentius
Copy link
Contributor Author

I've added a generic function each to for both sum and product as a type-level assert of applicability and one test computation as a sanity test for the correct result. I can add more samples, but I think I'd be testing Mul and Add more than sum and product.

@cuviper
Copy link
Member

cuviper commented Apr 12, 2018

Yeah, that level of testing looks like plenty, thanks.

bors r+

bors bot added a commit that referenced this pull request Apr 12, 2018
25: impl std::iter::{Sum, Product} for Ratio r=cuviper a=Emerentius

Fixes #3

I only implemented `Sum` and `Product` for `Ratio<T>` and `&Ratio<T>` rather than any `U, where Ratio<T>: Mul<U, Output = Ratio<T>>`. Summing for example an iter of `T` directly into a `Ratio<T>` does not seem like particularly useful or critical functionality.

Co-authored-by: Emerentius <emerentius@arcor.de>
@bors
Copy link
Contributor

bors bot commented Apr 12, 2018

Build succeeded

@bors bors bot merged commit 97968eb into rust-num:master Apr 12, 2018
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.

2 participants