Currency rate: always display minimum 4 significant decimals or 2 trailing zeros if the number is integer

[rudyhuyn]

Fixes #252

Converter rounds rate to 4 decimal places, therefore currency ratio of low valued units such iranian rial or indonesian rupiah compared to highly valued units like US dollars displayed incorrectly.

Left: before, Right: after.

Description of the changes:

Modify how we round the currency rate. Instead of always cropping the number after 4 decimals, we compute how many decimals are needed to display the currency rate with minimum 2 meaningful decimals.

Number of decimals to display: Max(4, (int)-Log10(currencyRate) + 2)

Replace also static_cast<int>(ratio * static_cast<int>(scale)) / scale; to support bigger scales.

How changes were validated:

• Unit tests added
• Manual tests in English and French

[jlaanstra]

Looks like we used to do 4 and now arbitrarily do 2 for small numbers. I think we should be consistent and either always use 4 or 2 or display all digits.

[rudyhuyn]

Looks like we used to do 4 and now arbitrarily do 2 for small numbers. I think we should be consistent and either always use 4 or 2 or display all digits.

Not really.

The title is probably not enough clear. This code will always display 4 decimals minimum, but if the number looks like 0.0003 or 0.0000, it will add more digits to display minimum 2 meaningful digits:

Source	Current (always 4 decimals)	Diff (from 4 to X, in order to always display minimum 2 meaningful digits)
12	12.0000	12.0000
12.232	12.2320	12.2320
12.232124	12.2321	12.2321
0.1231	0.1231	0.1231
0.0231	0.0231	0.0231
0.005311	0.0053	0.0053
0.00014311	0.0001	0.00014
0.000064314	0.0000	0.000064
0.0000000064314	0.0000	0.0000000064

[jlaanstra]

@rudyhuyn my point would be that 2 meaningful digits is random. I would say that if we decide to round, we should do so consistently, and always use the same amount of meaningful digits.

[rudyhuyn]

@rudyhuyn my point would be that 2 meaningful digits is random. I would say that if we decide to round, we should do so consistently, and always use the same amount of meaningful digits.

Got it! It's only a constant to change, I will do it later!

[rudyhuyn]

@jlaanstra If the value is exactly 0.00001, should we display 0.00001 or 0.00001000?

[jlaanstra]

@rudyhuyn if you decide to display 4 meaningful digits, then 0.00001000 would be correct. However I think 2 is more useful, but in that case 12.2320, becomes 12.23.

[rudyhuyn]

12.23 isn't enough precise, especially for a currency rate.

Diff updated to manage 4 meaningful digits! Thanks for your feedback!

I'm not sure 0.0001000 will be helpful, but if it's something you want to do that, DecimalFormatter will need a refactoring.

[grochocki]

I'm not sure 0.0001000 will be helpful, but if it's something you want to do that, DecimalFormatter will need a refactoring.

We should not show trailing zeros. I believe precision with zeros matters more for scientific applications, but for currency, we should trim them.

[rudyhuyn]

I'm not sure 0.0001000 will be helpful, but if it's something you want to do that, DecimalFormatter will need a refactoring.

We should not show trailing zeros. I believe precision with zeros matters more for scientific applications, but for currency, we should trim them.

the current diff would work in this case!

[blurbusters]

Consistency is not the right tool for this particular job:

1. The big numbers should be Easy Info -- e.g. "0.00" or the most accepted common number of digits (e.g. 2 digits for most currencies) -- whatever cashiers need to do to their job. Using Calc on a Surface at a charity booth that accepts both USD and CAD. Cashiers don't need to know more digits than needed.

2. The tiny number should be Details Info with sufficient enough number of digits to display at least 2 significant digits, even if it requires a large number of digits after the decimal. e.g. "0.000024" -- basically it tells you 1 unit of a currency that it's worth something but less than a penny -- important context that is not quickly answered by "0.00" nor "0.000" since some users get confused by that.

I thereby agree with the earlier @rudyhuyn approach, but perhaps change to 4 meaningful digits (followed by zero trim) for the tiny number.

Much more logical approach matching real-world "need to know" information use cases.

Going back to consistency in this particular case, is a regression, full stop.

The problem is that this original tweak away from consistency (for an excellent good reason) was incomplete because hardcoded-4 does not reveal enough digits in some currency conversions like this one. To fix this, the @rudyhuyn variable-digits tweak solves this closer to the intent of "good reason" away from consistency.

Thus, easy info could stay hardcoded-2 as cash register standard or accounting standard -- and details shall go "a few significant digits visible" as currency infoboard standard regardless of how many digits is needed in order to display all important nonzero digits.

For trailing zeros, having too many zeros after significant digits is a lesser evil than having all zeros. That said, smart trimming is recommended best-practice for improved readability, but only down to minimum length (e.g. 2 digits). So that things like "$0.1000" becomes "$0.10" instead of "$0.1". While "$0.00002450000" becomes "$0.0000245". Devil is in the details, but recommended practice definitely diverges from hardcoded-2 when displaying per-unit-currency numbers in a "1 unit equals X" details section. Also, the currency rule for minimum number of decimal digits displayed on a cash register will vary from country to country, so query the system for that because it ain't always 2.

If consistency is an unavoidable mantra, simply go for algorithm consistency. Get exactly a minimum of 4 significant decimal digits regardless of how many zeros after the decimal point. And then smart-trim according to above rules, for both numbers. Still variable number of digits, but at least you've achieved algorithmic consistency for both top & bottom numbers.

Note: I've worked as an employee for a Canadian bank, working as an app developer. LinkedIn Proof. Mic drop.

[HowardWolosky]

@grochocki / @jlaanstra - @mdrejhon has a super compelling argument here, and has fully convinced me. I think that if we're going to make a change here, let's do it once and do it right. I love @mdrejhon's suggestion that we keep the big number at two-significant digits, and the detail number to be as many as necessary to have two significant digits (with a minimum of two digits).

[rudyhuyn]

we keep the big number at two-significant digits, and the detail number to be as many as necessary to have two significant digits (with a minimum of two digits).

The current version of the application up to 4 decimals:

My initial PR was: display up to 4 decimals (with a minimum of two significant digits)

Following feedback, I changed it to: display minimum of 4 significant digits.

It's very easy to change the behavior of this code using these 2 contants:

static constexpr int FORMATTER_RATE_MIN_DIGIT_COUNT = 4;
static constexpr int FORMATTER_RATE_MIN_SIGNIFICANT_DIGITS = 4;

It looks like the behavior you are describing is:

static constexpr int FORMATTER_RATE_MIN_DIGIT_COUNT = 2;
static constexpr int FORMATTER_RATE_MIN_SIGNIFICANT_DIGITS = 2;

@HowardWolosky and @mdrejhon, is it what you want?

value	current version of the app	New version
12	12	12
12.2321	12.232	12.23
12.232	12.232	12.23
12.0000007132	12	12 or 12.00000071?
12.232124	12.2321	12.23
0.1231	0.1231	0.12
0.0231	0.0231	0.023
0.005311	0.0053	0.0053
0.00014311	0.0001	0.00014
0.000064314	0.0000	0.000064
0.0000000064314	0.0000	0.0000000064

[grochocki]

@rudyhuyn I don't think we want to lose precision for "big" numbers in currency converter in the same way that @mdrejhon suggests is ok for the cash register scenario. I would expect:

Value	Desired
12	12
12.2321	12.2321
12.232	12.232
12.0000007132	12
12.232124	12.2321
0.1231	0.1231
0.0231	0.0231
0.005311	0.0053
0.00014311	0.00014
0.000064314	0.000064
0.0000000064314	0.0000000064

[rudyhuyn]

@rudyhuyn I don't think we want to lose precision for "big" numbers in currency converter in the same way that @mdrejhon suggests is ok for the cash register scenario. I would expect:

Value Desired
12 12
12.2321 12.2321
12.232 12.232
12.0000007132 12
12.232124 12.2321
0.1231 0.1231
0.0231 0.0231
0.005311 0.0053
0.00014311 0.00014
0.000064314 0.000064
0.0000000064314 0.0000000064

OK! It will be easy, it was the behavior of my first commit, let me rollback the last change!

[blurbusters]

Although the behaviour is better, I best clarify.

The exact nuances is sometimes a debate even within banks, investment businesses, and sometimes (For the small number, currency infoboards) we just displayed exactly what the quote provider gave for a FX quote. And reformatted to standard 2 digit with rounding for the transaction (accounting, cash register, bank statement, etc).

Nontheless, there was always a distinction between an infoboard (e.g. stock trading, currency boards, realtime quotes) and an accounting format of a transaction.

Sometimes data feeds had odd data like 12.00000000000000000713 which should be trunctated. If your Quote provider guarantees that doesn't happen, just display the quote feed directly for the small details number.

Thusly,

• Preserve all digits to left of decimal
• Keep X significant decimal digits (e.g. 4, 6 or 8...)
• Now you've got the interim format
• For details/quoteboard/infoboard numbers, display interim format directly
• For accounting format numbers, reformat interim format to exact number of digits (most regions will be two decimal digits for most currency regions. Querying the OS will tell you the standard count, which you should use)

Although the number of digits chosen (4) may be somewhat arbitrary, you can decide to avoid that -- just display quotefeed quote number for the details number. Stock quote trading systems will usually display ALL the digits in the feed, but sometimes there's internal trunctation of unnecessary digits (e.g. 12.0000000000000001 or whatever quotefeed artifacts caused by the decimal storage formats (whether it be IEEE 754 or other decimal standard) that sometimes creates "artifacts" like those.

Either way, this is a commonly chosen behavior in the industry:

Entry	Claimed-Current	Big-Number (Accounting)	Small-Details-Number (Infofeed)
12	12	12.00	12.00
12.2321	12.232	12.23	12.2321
12.232	12.232	12.23	12.232
12.0000007132	12	12.00	12.00
12.232124	12.2321	12.23	12.2321
0.1231	0.1231	0.12	0.1231
0.0231	0.0231	0.02	0.0231
0.005311	0.0053	0.00	0.005311
0.00014311	0.0001	0.00	0.0001431
0.000064314	0.0000	0.00	0.00006431
0.0000000064314	0.0000	0.00	0.000000006431

Do not display "0" instead of "0.00".
The user needs to know that it's under a penny.
"0.00" is more informative than "0".
Then even if the user is still confused looking at "0.00", they can see the "1 ACMEcoin = USD 0.00000000001363" in the details at the bottom.

If this produces any feature regressions that we have to find a compromise path, please describe the regression that occurs if this is done, and I'll think in the mentality of how banks/investments applies a bugfix towards such a situation.

[HowardWolosky]

cc @grochocki

Before we waste any more of @rudyhuyn's time here with changes, it seems that we need to have a bit more discussion on this topic to make sure that we're making the right change. I'm going to lock this PR for the time being so that we don't have the conversation split between the PR and the Issue. Once we have what appears to be a mutually agreed upon desired outcome, I'll unlock this so that Rudy can pursue the appropriate fix.

[grochocki]

Unlocking after approving #252. See details there for more information on direction for this.

[rudyhuyn]

Modified to use the specified format. Ready for review

[sanderl]

I built and tested this change locally. The code looks good to me. Thanks for adding the unit test.