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

Absolute value #266

Open
mattfbacon opened this issue Jan 19, 2023 · 6 comments
Open

Absolute value #266

mattfbacon opened this issue Jan 19, 2023 · 6 comments

Comments

@mattfbacon
Copy link

mattfbacon commented Jan 19, 2023

I would like to have a way to make a BigInt into its absolute value in place, i.e. taking &mut self. Seems like a good approach would be to add an abs method to Sign which is just

match self {
  Self::NoSign => Self::NoSign,
  Self::Minus | Self::Plus => Self::Plus,
}

and then in BigInt it would be

self.sign = self.sign.abs();

If this approach is acceptable I can make a PR.

@cuviper
Copy link
Member

cuviper commented Jan 23, 2023

I don't think we need anything added to Sign, just:

impl BigInt {
    fn bikeshed_abs_in_place(&mut self) {
        if self.sign == Sign::Minus {
            self.sign = Sign::Plus;
        }
    }
}

This seems ok, but the hard part is to find a good name for it. :)

If you have ownership, you can use BigInt::from(bigint.into_parts().1), and maybe that deserves fn into_abs(self). From there you can also fake in-place on &mut by using mem::take to steal the value and then write it back.

@mattfbacon
Copy link
Author

Okay, those approaches seem better to me. I especially like the BigInt::from approach.

Anyway, assuming the implementation you gave, do you think that BigInt::abs is not a good name?

@cuviper
Copy link
Member

cuviper commented Jan 24, 2023

There is already an abs(&self) through the Signed trait:
https://docs.rs/num-bigint/latest/num_bigint/struct.BigInt.html#method.abs

@mattfbacon
Copy link
Author

Ah, how about make_abs then.

@cuviper
Copy link
Member

cuviper commented Jan 24, 2023

Yeah, make_abs sounds ok. There's some precedent in std with make_ascii_upper/lower_case, make_contiguous, and make_mut.

@youknowone
Copy link
Contributor

instead of &mut, how about fn into_abs(self)?

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

No branches or pull requests

3 participants