Changes to make Decimal serialization more deterministic with decimal places

[barticus]

Summary of the changes (Less than 80 chars)

• Change DecimalType serialization to use the "F" specifier rather than "E" which lost precision of decimal places once number had more than 6 significant digits
• Also added an optional override for the decimal type to allow specifying of the number of decimal digits

Addresses #1929

[barticus]

Hi @michaelstaib / @PascalSenn , this was a little more complex than i originally thought:

1. The "F" specifier is more deterministic than "G", but by default only maintains 2 decimal places.
2. As current behaviour would have allowed up to 6 decimal places, i've set the formatter, by default, to be "F6". This can be controlled by an overloaded constructor - let me know if theres a better way of exposing this
3. To allow me to use the new variable for formatting, the method needed to become non-static.

So I think this will result in no breaking changes, but fix the issue as reported.

[michaelstaib]

Not sure if we than should use F. I will have a look at this later.

[barticus]

Thanks Michael.
To me, F is a better fit than G. You can pull this branch and change the specifier to see the impacts it has.
But maybe to stop breaking changes, it is changed to G as the default, but if NumberDecimalDigits is specified, then use F.

[barticus]

Hey @michaelstaib , anything further you want me to do on this one?

[michaelstaib]

Sorry for the wait will look at this later

[bfink87]

@michaelstaib Is this going to be merged into the 10.5 preview?

[michaelstaib]

Yes, but I have to rework it first

[michaelstaib]

But we will fix the issue

[bfink87]

Thanks for the quick reply. Is there anything I can do to help? This bug is causing an issue in a project I'm working on.

[barticus]

Hi @bfink87,
You can copy this modified type into your project and use it instead of the inbuilt type if you need to work around the issue until the fix is available.

@michaelstaib it's disappointing you didnt ask for help seeing as I wrote this patch with guidance from yourself and Pascal, and even sought feedback with no response.

[michaelstaib]

@barticus we are at the moment focused on version 11, but we will merge this one probably this week. We will change it to G as discussed with you.

[michaelstaib]

@barticus I have removed NumberDecimalDigits since serialization/deserialization, in my opinion, should essentially lead to the same value. That was also my main hold up on this one. I will merge this one now and integrate it into preview-8. Sorry for the wait on this one but at the moment we are really focused on version 11 which really takes a lot of my attention right now. We try to get 10.5 out this week.

[michaelstaib]

@barticus G does not contain only two digits; it will contain all significant digits.

[sonarcloud]

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
1 Code Smell

79.6% Coverage
1.4% Duplication

The version of Java (1.8.0_252) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

[barticus]

Hi @michaelstaib,
Thanks for finishing this off.
Yes I had read the docs but I think was using G with a precision specifier and in that case it will revert to scientific notation if the significant digits is greater than the specified precision which wasn't a good thing to me.

Will v11 allow users to specify the precision they want on certain types?

[michaelstaib]

@barticus not sure about this. What is the benefit of having zeros at the end?

[barticus]

@michaelstaib it's not about zeroes at the end, it's about matching the precision of your data types, especially DB types. If you only want to accept currency values up to 2 decimal places, it would be nice to be able to handle this at the type layer.
But if you have a suggestion/opinion on this, id be interested, I haven't been able to find what should be done for cases like this, e.g make a unique type for each precision, or handle it as a business logic check or some other middleware.
Thanks