search

kornelski / rust-rgb

struct RGB for sharing pixels between crates
https://lib.rs/rgb
MIT License
97 stars 19 forks source link

Minimal Trait Bounds + `IntoIterator` trait bound #90

Closed ripytide closed 2 months ago

ripytide commented 2 months ago

I think I've changed my mind regarding using num-traits, I think it's better to offer the minimal number of trait bounds on the two Pixel traits and the PixelComponent trait since if a user requires those behaviours then they can always bound their functions with num-traits but on the other hand if a user has a type that does not implement one of those behaviours they cannot ever implement HomPixel or HetPixel even if they could provide all of the relevant methods. It also saves us a dependency as you mentioned.

I've also added a IntoIterator super-trait bound to HomPixel since that is a relevant trait that should never not be able to be implemented.

kornelski commented 2 months ago

I wonder if pixel traits should have more restrictive requirement like Copy + 'static. It would be weird if someone made RGB<&u8> by accident (e.g. by using [arr].iter().collect() instead of into_iter()).

ripytide commented 2 months ago

I think the compiler should tell them about it if they end up with a type they didn't expect such as Rgb<&u8> so I don't think that's to big of an issue.

ripytide commented 2 months ago

Actually, now I remember I think I have run into this kind of scenario before and adding a 'static bound was necessary but getting to that point was really confusing even with the compilers error messages so maybe it is worth it like you say, hmm.

ripytide commented 2 months ago

I was going to give a counter-argument to Pixel: Copy of using a heap-allocated integer type like BigInt but then PixelComponent: Copy is already a given so I don't see why we can't add Copy to Pixel also.