JavaMoney/jsr354-ri

Default printing and parsing not symmetric anymore

odrotbohm opened this issue · 8 comments

odrotbohm commented

This test case works on 1.3 but fails on 1.4 snapshots:

Money money = Money.of(1.2, "EUR");

assertThat(money).isEqualTo(Money.parse(money.toString()));

This seems to be caused by the toString() method using a formatter based on Locale.getDefault() which obtains a machine specific local (German in my case). parse(…) however uses DEFAULT_FORMATTER that does not consider any Locale at all.

odrotbohm commented

Using 1.4 seems necessary if you want to work with JDK 11 or better as the fix for #208 only seems to be included into 1.4.

keilw commented

I discussed this with @atsticks last week and he was under the impression it worked for some, but maybe another PR or fix changed the behavior. We definitely plan this for the MR1 which is 1.4, should this or other issues be a major problem for some big users of JavaMoney or Spring, please let us know, we might get them into a service or fix release like "1.3.1". 1.4 is meant for MR1 which has to go through the JCP EC in the course of the next couple of weeks.

atsticks commented

As suggested the issue is dependent on the default locale currently set. My proposal is to

  1. Use a non localized default toString format as in previous versions (CCC xx.xx), where xx.xx equals to the BigDecimal format, rounded to a precison of 2..
  2. By setting the org.javamoney.moneta.useJDKdefaultFormat config property toString can be used to use the JDK based formatting (using the system's default locale).
  3. In both cases formatting/reparsing is ensured (I will add corresponding tests).
stokito commented

xx.xx equals to the BigDecimal format, rounded to a precision of 2..

This may cause another problem with symmetry: we can loose precision if we have digits lower than 2 e.g. 42.12345. If we made some calculations then stored them to CSV file then we would like to read it by some another program and process without loosing precision (e.g. multiply) and only format them for a user.

toString can be used to use the JDK based formatting (using the system's default locale).

I'm afraid this behavior may be unexpected for developers. Consider a situation:

  1. One dev from US wrote some part of code (CSV export) and think that it will always formatted with a point. In fact he even didn't know that internally it will be used a system locale (i.e. US)
  2. Another dev from Spain wrote another part of the same system and he wanted to see amount formatted with a comma , so he changed the config. He is happy but meanwhile CSV export now is accidentally changed too and CSV files are broken because the comma is used as columns delimiter.

Yes, off course they both should read JavaDocs but to avoid such situations it should be clearly separated in API: toString() which always uses the same canonical format and something like toLocalString() or toString(Locale) which uses a locale. Something similar to Locale.getDisplayName(Locale inLoc).
Or like in JavaTime API: the toString() of Instant, LocalDateTime is always in canonical form and you have to use a formatter explicitly.

Another reason to avoid this is because it affects a performance. Imagine a CVS export with thouthands of lines and for each Money.toString() it will be checked some additional config and called a formatter.

Please take a look on the comment #307 (comment)

keilw commented

While the behavior of NumberFormat may be different, that proposal matches that of JSR 385. The default toString() is always locale-agnostic while specific cases also on the UI require to use some kind of builder for a more sophisticated formatting.

Let's also see, if @odrotbohm has any opinion on the possible solutions.

atsticks commented

This is a rather complex tasks, but I was able to fix it. Things to remember:

  1. The JDK also parses numbers such as 12-54.234 (result is 12). So to decide what kind of input is an amount's number and what is not requires probably more logic than we have as of today.
  2. The JDK internally uses the non breakable spaces to differentiate between number and currency part. Ordinary spaces are used as separator for any currency and parsing of the number stops at an ordinary space char, regardlessd of the pattern. This makes replacing all with ordinary spaces more complicated than needed (but it was possible).
  3. Formatting and parsing is never 100% symmetrical, since we do also accept non-breaking spaces in addition to ordinary spaces, but will output any formatted strings with ordinary spaces in all cases.
atsticks commented

Fixed.

keilw commented

Is there a Unit Test for it?