Skip to content

Implement Median for slice types with macro#274

Merged
rustaceanrob merged 1 commit intomasterfrom
median-trait-1-16
Jan 17, 2025
Merged

Implement Median for slice types with macro#274
rustaceanrob merged 1 commit intomasterfrom
median-trait-1-16

Conversation

@rustaceanrob
Copy link
Collaborator

Attempting to remove the repeated logic here, but also trying to avoid the trait hell that is required for implementing Median in a generic way. I extracted the logic into a macro which is reusable for any type of interest to implement Median

cc @nyonson

@nyonson
Copy link
Collaborator

nyonson commented Jan 16, 2025

Dang is there really no good trait combo to describe "numbers"? I am a macro noob so I am gonna just play around with this one to learn it.

@rustaceanrob
Copy link
Collaborator Author

rustaceanrob commented Jan 16, 2025

I would say a number is described by the following T: Copy + PartialOrd + Default + std::ops::Add<Output = T> + std::ops::Div<Output = T> + From<u8>, which is horrible. I prefer the macro because the arithmetic is clear and there are no weird From::(2) type shit going on

ref: https://internals.rust-lang.org/t/please-can-we-add-a-basic-num-trait/16449

@nyonson
Copy link
Collaborator

nyonson commented Jan 16, 2025

I would say a number is described by the following T: Copy + PartialOrd + Default + std::ops::Add<Output = T> + std::ops::Div<Output = T> + From<u8>, which is horrible. I prefer the macro because the arithmetic is clear and there are no weird From::(2) type shit going on

ref: https://internals.rust-lang.org/t/please-can-we-add-a-basic-num-trait/16449

Wow yea. I guess we could shove that in an alias and boil it down to just the bare minimum requirement for the function (which could be what you have already), but we till wouldn't get away from the weird From::(2) stuff.

@rustaceanrob
Copy link
Collaborator Author

Either From::(2) or 2.into(), which I find equally strange. With the macro all of the abstraction is in defining the trait and the actual implementation is more legible imo.

@nyonson
Copy link
Collaborator

nyonson commented Jan 16, 2025

Yea, I still find the macro syntax wierd, but that is me just not knowing. I think the macro is essentially doing what the compiler would too for the trait bound?

Copy link
Collaborator

@nyonson nyonson left a comment

Choose a reason for hiding this comment

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

Concept ACK 73beeaa

I am gonna take some time to finally learn macros

@rustaceanrob
Copy link
Collaborator Author

rustaceanrob commented Jan 16, 2025

think the macro is essentially doing what the compiler would too for the trait bound?

Yeah, the compiler will re-implement the trait for each type that ends up using it

@rustaceanrob rustaceanrob merged commit 04a038b into master Jan 17, 2025
14 checks passed
@rustaceanrob rustaceanrob deleted the median-trait-1-16 branch January 17, 2025 02:48
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