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

please add get original value funcion #237

Closed
piyongcai opened this issue Jul 14, 2021 · 12 comments
Closed

please add get original value funcion #237

piyongcai opened this issue Jul 14, 2021 · 12 comments

Comments

@piyongcai
Copy link

in current decimal,IntPart Function to get decimal IntPart value

// IntPart returns the integer component of the decimal.
func (d Decimal) IntPart() int64 {
	scaledD := d.rescale(0)
	return scaledD.value.Int64()
}

BUT, IN SOM TIME, NEED TO GET ORIGINAL VALUE

eg: PROTOBUF , you need define Decimal message, and you need convert between proto.Decimal and decimal.Decimal

so, it's really neet to to get original value function,

To get this purpose, I copied decimal code, and Add Int64Value function

PLEASE ADD

PLEASE ADD

PLEASE ADD

// Int64Value returns the original  value.
func (d Decimal) Int64Value() int64 {
	return d.value.Int64()
}

the follw code is my proto Decimal define:

// Example: 12345.6789 -> { units = 12345, nanos = 678900000 }
message Decimal {

  // Whole units part of the amount
  int64 val = 1;

  // Nano units of the amount (10^-9)
  // Must be same sign as units
  int32 exp = 2;
}

convert between decimal.Decimal and proto.Decimal

1、decimal.Decimal -> proto.Decimal

func FromDecimal(d decimal.Decimal) *proto.Decimal {
	return &Decimal{
		Val: d.Int64Value(),
		Exp: d.Exponent(),
	}
}

2、proto.Decimal -> decimal.Decimal

func (x proto.Decimal) ToDecimal() decimal.Decimal {
	return decimal.New(x.Val, x.Exp)
}
@piyongcai piyongcai changed the title place add get original value funcion please add get original value funcion Jul 14, 2021
@mwoss
Copy link
Member

mwoss commented Jul 21, 2021

Hi @piyongcai! That sounds like a good idea, I will add your request to my backlog.
In the meantime, you can use the Coefficient method to simulate the behaviour you want.
Example:

d := NewFromFloat(5.45)
d.Coefficient().Int64()

@piyongcai
Copy link
Author

piyongcai commented Aug 20, 2021

// Coefficient returns the coefficient of the decimal.  It is scaled by 10^Exponent()
func (d Decimal) Coefficient() *big.Int {
	d.ensureInitialized()
	// we copy the coefficient so that mutating the result does not mutate the
	// Decimal.
	return big.NewInt(0).Set(d.value)
}

in Coefficient function, call big.NewInt(0).Set(d.value), this isn't a good idea.

@mwoss
Copy link
Member

mwoss commented Aug 23, 2021

@piyongcai What do you mean by "this isn't a good idea"? Could you elaborate on that?

@clstb
Copy link

clstb commented Aug 29, 2021

Had the same problem and drafted a PR for this @ #241.

@mwoss
Copy link
Member

mwoss commented Aug 29, 2021

@clstb Maybe I'm still asleep, but what is the difference between your proposed implementation and an example I've showed earlier d.Coefficient().Int64()?

@clstb
Copy link

clstb commented Aug 29, 2021

You were not asleep but I was apparently. I plugged your workaround into my test case and it errored. I thought this It is scaled by 10^Exponent() was the culprit since the value I got mismatched exactly by that.

Edit: After retrying it behaves how it should. Should have looked more carefully in the beginning 👍

@mwoss
Copy link
Member

mwoss commented Aug 29, 2021

So I guess the most suitable option here would be to extend API by adding the CoefficientInt64 method or something similar then.
Also, I still would love to know what @piyongcai had in mind by telling "this isn't a good idea", it might be helpful tho.

@piyongcai
Copy link
Author

// Coefficient returns the coefficient of the decimal.  It is scaled by 10^Exponent()
func (d Decimal) Coefficient() *big.Int {
	d.ensureInitialized()
	// we copy the coefficient so that mutating the result does not mutate the
	// Decimal.
	return big.NewInt(0).Set(d.value)
}

Because this method has a very low performance。
in fact, the flow method is better way.

// Int64Value returns the value.
func (d Decimal) Int64Value() int64 {
	return d.value.Int64()
}

In the usage scenario of protobuf, it is generally used for amount calculation, and the maximum decimal number is set to 4 bits.The message is defined as follows.

// Example: 12345.6789 -> { units = 12345, exp = 4} = 1.2345
message Decimal {

  // Whole units part of the amount
  int64 val = 1;

  // Nano units of the amount (10^-9)
  // Must be same sign as units
  int32 exp = 2;
}

I can convert between the two in the simplest way

@mwoss
Copy link
Member

mwoss commented Sep 9, 2021

Yup, so let's expose new public method called CoefficientInt64 that would explicitly return coef as int64 value. It should resolve your issue and probably be helpful for others. I will craft the PR later today.

@piyongcai
Copy link
Author

thanks

@mwoss
Copy link
Member

mwoss commented Sep 10, 2021

#244 has been merged to main branch.
@piyongcai @clstb can you take a look if this is something you had in minds? :D

@piyongcai
Copy link
Author

piyongcai commented Sep 21, 2021

3ks,it's works

@mwoss mwoss closed this as completed Oct 13, 2021
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 a pull request may close this issue.

3 participants