servo / euclid

Geometry primitives (basic linear algebra) for Rust
Other
462 stars 102 forks source link

Vector missing the Sum trait #469

Closed mikepurvis closed 3 years ago

mikepurvis commented 3 years ago

Hey, I'm a new rustacean who did Advent of Code this year in Rust and used the Euclid crate for several of the puzzles. One of the ergonomic quirks I hit was that the Vector types didn't work with sum(), so I ended up having to use a fold instead with a trivial closure:

https://github.com/mikepurvis/advent-of-code/blob/5d9ef10d566dc012f41ccadc5dbcd3474c7c8e19/2020/day-24/src/main.rs#L43-L54

I believe this should be a small change and can submit a PR for it if there's interest. I would apply the change to basically all existing structures which have the AddAssign trait:

https://github.com/servo/euclid/search?q=AddAssign

nical commented 3 years ago

Hi, sounds good to me! Euclid has a no-std feature so if you would like to submit a PR, please use core::iter::Sum instead of the std one so things keep working in that configuration. You would add a Sum implementation to all types that implement AddAssign where the parameter is the same type (for example no Sum for point types since point + point is disallowed.

mikepurvis commented 3 years ago

Okay! I got started by implementing it for Angle, and that was fairly trivial, but in order for it to work with iter().sum() as well as iter().copied().sum(), it needed to have a second Sum<&Angle> impl, and this necessitated also adding an Add<&Angle> impl. All this works fine, but before I proceed with the rest of the types, I wanted to check:

My work so far is here: https://github.com/servo/euclid/compare/master...mikepurvis:sum-traits

mikepurvis commented 3 years ago

Closing in favour of #470.