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

create Abs trait for absolute value #41

Merged
merged 1 commit into from
Jun 25, 2018
Merged

Conversation

droundy
Copy link
Contributor

@droundy droundy commented Jun 25, 2018

I wrote this very quickly, so please do check that I haven't missed anything. It's just got the one example as a test.

Copy link
Owner

@paholg paholg left a comment

Choose a reason for hiding this comment

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

Looks good.

We could also add an impl for i128, but that was only fairly recently stabilized, so it's probably best to leave off for now if there's no demand for it.

It also occurs to me that we could do special case implementations for this and other functions on primitives, so they can be called without the traits in scope.

For example,

impl<U> $System<i32, U> {
    fn abs(self) -> Self { $System::new(self.value_unsafe.abs()) }
}

But that can be left for another time.

@paholg paholg merged commit 339614f into paholg:master Jun 25, 2018
@droundy
Copy link
Contributor Author

droundy commented Jun 25, 2018

Do you do special case implementations like that for any other methods? It seems potentially very appealing, but I'd think that consistency would be key.

@paholg
Copy link
Owner

paholg commented Jun 25, 2018

I don't think so. I think it just hadn't occurred to me before.

I think it would be worth adding for at least anything we have a trait for, as well as any other methods that exists on primitives.

I would happily accept such a PR, or may make one myself at some point.

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.

None yet

2 participants