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

Implement Median for slice types with macro #274

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Conversation

rustaceanrob
Copy link
Owner

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
Contributor

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
Owner 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
Contributor

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
Owner 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
Contributor

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
Contributor

@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
Owner 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