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

Decimal implements Deref #48

Merged
merged 1 commit into from
Mar 8, 2024
Merged

Decimal implements Deref #48

merged 1 commit into from
Mar 8, 2024

Conversation

Vam-Jam
Copy link
Contributor

@Vam-Jam Vam-Jam commented Oct 17, 2023

This PR implements Deref for Decimal, to remove the need to write .as_f64/.0 to access the inner value and its functions.

The original PR for decimals mentions (#34 (comment)) not implementing it based on rust docs guidance. At the moment, the rust community is still trying to define what a smart pointer is (rust-lang/rust#91004).

In my use case, I commonly just need to just work with the inner value of Decimal, without really caring about the wrapper, which results in a lot of .0 or .as_f64. I'd figure other developers might be in the same boat. I do not see any real downside with implementing Deref in this instance.

@Vam-Jam
Copy link
Contributor Author

Vam-Jam commented Oct 17, 2023

I have signed the CLA!

@jmaargh
Copy link

jmaargh commented Oct 21, 2023

FYI: Some new guidance on Deref is currently under review which moves away from the "smart pointer" lingo: rust-lang/rust#110340

Copy link
Contributor

@jbourassa jbourassa left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

@jbourassa jbourassa merged commit f26c8a9 into Shopify:main Mar 8, 2024
@Vam-Jam Vam-Jam deleted the decimal-deref branch March 8, 2024 15:56
@jbourassa jbourassa mentioned this pull request Mar 8, 2024
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.

3 participants