Adding a space between the currency code and the balance

[ziadOUA]

Hello,
I modified a bit the Utils.java code to put a space between the currency code (CAD) and the balance (-> CAD 50.00). It works as intended on zCard.
This is my first Hacktoberfest contribution :)

[obfusk]

Thanks. Though you're a bit too early for October :)

Are we sure there are no locales where the space should be on the left side? Or that otherwise behave differently in a way that matters here? And what about word wrapping?

I do agree "USD 5.00" probably looks nicer, but I think I'd prefer "€5.00" w/o the extra space, though not strongly.

Seems like a simple enough change, but we should make sure nothing unexpectedly breaks.

[ziadOUA]

I only adds a space for the currency codes -> USD 5.00...
.. but doesn't add it to the currency symbols -> £5.00
It depends on the locale, per example in French, we would write 5.00 £, and not £5.00.

[obfusk]

Right. I overlooked you're only replacing the codes. Makes sense.

That does mean that in French, you'd get 5,00 XXX with an extra space now. Maybe only replace it at the start then?

[ziadOUA]

Could adding .trim() to the end of return formattedCurrency.replace(currency.getCurrencyCode(), currency.getCurrencyCode() + " ") remove the extra space ?

[TheLastProject]

I have to say I'm a bit confused about this change because it is basically saying "the currency formatting code, which uses official Android methods, is wrong" and I think that is a pretty big claim that should come with examples and explanations on why this change is better without breaking anything, especially because Android uses the official Unicode standards. Assuming every country adds a space seems to be a pretty big assumption to me. I also feel this change makes Catima's behaviour less consistent.

Before:

€5.00
CA$5.00
ARS5.00

After:

€5.00
CA$5.00
ARS 5.00

And if I look at English Wikipedia, they write no space after ARS (but add a $): ARS$350 (but for example Dutch Wikipedia writes 381,0 ARS and French writes 3,12 ARS which is also way different from the behavior in this MR).

To make it even more fun, if I put my phone language to Dutch, the original code already adds one space (ARS 5,00), but your code adds another space causing two spaces:

So I'm not convinced this MR is fixing anything. It looks more like it is just changing things based on personal preference without taking official standards into consideration and introduces new bugs. Even if this bug gets fixed, I think it's fundamentally a bad idea to mess with the .format() output.

Sidenote: Looking into this more deeply I did notice that Utils.formatBalance does a call to NumberFormat.getCurrencyInstance() which "Returns a currency format for the current default FORMAT locale." So maybe there is a bug there and we should be passing a locale to getCurrencyInstance to format it correctly for the user's language?

[ziadOUA]

Hello @TheLastProject,

This PR has neither to do with personal preference, nor pretentious positioning, because the ISO-4217 norm (Standard that defines codes for the representation of currencies) doesn't specify the placement of the currency code.

I added a space to make the amounts easier to read (ARS5.00 is harder to read, because the S resembles the 5, which is palliated by the space -> ARS 5.00).

For the double space behavior in Dutch, We could try to fix it ?

[obfusk]

I think the issue here is that correct currency formatting is actually pretty complex since you have to take all possible locales into account. Which is why it's great if we don't need to worry about all that detail and can just rely on Android/Java to do "the right thing".

I agree that "ARS 5.00" is easier to read than "ARS5.00". But if we want to modify this we need to make sure we handle it right and catch all the possible edge cases correctly. There is also the "CA$5.00" vs "ARS 5.00" inconsistency we would introduce with this patch, which I'm not sure how to handle.

So at this point I agree with @TheLastProject that relying on Android/Java currency formatting seems less likely to accidentally break something than trying to improve this ourselves without knowing all the possible edge cases we need to take into account, even if I do agree that the idea behind the change makes sense as the space does increase legibility in the examples provided.

So maybe there is a bug there and we should be passing a locale to getCurrencyInstance to format it correctly for the user's language?

AFAIK it already does that correctly, with the default locale being the one the user has selected, though it may cause minor bugs when switching languages whilst the app is running (based on what I've seen trying to handle that case for another bug).