Money.toString() and Money.parse(…) not symmetric anymore

[odrotbohm]

Similarly to #319, the change made for #307 breaks print/parse symmetry without any kind of formatting involved. Assume you want to serialize (generally, not in a Java serialize way) a Money via toString() to eventually recreate it via Money.parse(…). Up until version 1.4, the following worked:

MonetaryAmount source = Money.of(1.23456, "EUR");
assertThat(Money.parse(source.toString)).isEqualTo(source);

This now fails with "Expected is <EUR 1.23> but was <EUR 1.23>". The reason for this is that as of 1.4 (#307) toString() now applies some default formatting, which I think is not only wrong, it's also highly misleading as you can see from the assertion message, as the source value is not 1.23, but 1.2345.

[keilw]

Can you explain why you mentioned MonetaryAmount.toString()? It is an interface and does not declare a toString() method, neither abstract nor default. So I guess it's both of these methods in the Money class?

[odrotbohm]

In the production code, I work with a MonetaryAmount but effectively it is a Money instance. I've updated the summary and description accordingly.

[keilw]

Also here please be patient until https://issues.sonatype.org/browse/OSSRH-68317 can be resolved by Sonatype because for some totally unknown and strange reason the same Sonatype user which never had problems to sync from Bintray while that was still around suddenly fails to deploy snapshots directly. Hope they can fix that soon, because no other "technical" or personal account I use for other projects and JSRs ever failed, only the "JavaMoney" one.

[keilw]

I did reopen that, because I found what caused it, starting with at least 1.4.0 or even before, Anatole had hardcoded the scale of ToStringMonetaryAmountFormat to 2 digits, hence toString() produces always a string with just 2 decimal digits as part of #307 among other changes.

It would be more or less a one-liner to use the scale of the BigDecimal, but that breaks a vast number of tests other than the 4 remaining failures in moneta-core (all of which seem a result of the modular ServiceLoader problem that only seems to affect those TestNG tests) and it would be a backward-incompatible change that may break production code.

Therefore I recommend making this customizable, but leaving the default number as 2 digits, a bit similar to overriding the default format as such via org.javamoney.moneta.useJDKdefaultFormat (there AFAIK no such scale limit exists). Offering a similar property like "org.javamoney.moneta.ToStringMonetaryAmountFormat.scale" or similar, which allows to override it and has some "smart" value (like -1) that takes the scale of the BigDecimal number.

@odrotbohm, @otaviojava what do you think? From a Maintenance Lead point I would say this is the most sensible change, and even if we never get to a 2.x release / whole new JSR, we might propose a MR2 or the already planned 1.5. It is possible, some issues planned only there like #277 could be related to this formatting cut-off.

And then we must document such backward-incompatible changes, giving users enough notice to migrate their code.

If that works here, and we can confirm, that #370 only affects a few tests but does not mess the runtime behavior (because the normal ConfigProvider etc. are all declared in a module-info, too) then all there is to do before we can release 1.4.3 is a solution for #353. Again destroying this feature is not acceptable, and I am confident, because the shell script works with curl instead of wget, that this can be solved, as usual help is appreciated if you like to get it out faster ;-)

[keilw]

There are two parts to this problem:

1. Using MonetaryAmount amount = Monetary.getDefaultRounding().apply(this); in all relevant toString() operations, not just Money, which in the given example already rounds down the amount to 1.23. The only one or two unit tests taking the rounding down away and formatting thisinstead those for currency code "XXX", e.g. testToString(). I am not that deeply familiar with currency exchange as to if "XXX", see X currencies (funds, precious metals, supranationals, other) required a special rounding, but as that's commonly a wildcard for "no currency" I guess it also depends on the particular transaction, hence I don't see why toString() should automatically round down. At most, if anybody knows a strong usecase, it could again be configured, but the default should be false here IMO.
2. applying a flexible/customizable scale in the ToStringMonetaryAmountFormat.

[keilw]

Actually the majority of test failures were pretty much the same, so avoiding the round-down plus an adjustment of the scale that also takes the maxScale andfixedScale `combination into consideration works pretty well.

Each of the three test cases now have a testParseFromToString()method. By applying MonetaryContext built via MonetaryContextBuilder.of().setMaxScale(2).setFixedScale(true).build() it is possible to enforce the fixed two-digits in toString(), I guess that should do.

@odrotbohm, @otaviojava please review.

[keilw]

Did not hear anything by either one, so I assume that is resolved.

[stefanzilske]

Hey, I just noticed, after upgrading from 1.4.2, that the previously applied default rounding to a fixed scale=2 in toString() is not applied anymore. We heavily rely on this, e.g. whenever we serialize a Money value to JSON. So it's a quite invasive breaking change.

Is there a way to apply this default rounding? I stumbled upon MonetaryConfig, but found no usable documentation on how to best do this in SpringBoot.

[keilw]

The hardcoded scale for toString() was what had to be fixed here, so adding it back would only be possible as an optional property, Please create an enhancement request here, and we'll see, if we could add something into javamoney.properties to customize it. In the meantime you should be able to enforce the scale of your choice by creating your Money like this:

Money money3 = Money.of(1234567.3, "EUR", MonetaryContextBuilder.of().setMaxScale(2).setFixedScale(true).build());
System.out.println(money3.toString());

prints "EUR 1234567.30".

[keilw]

@stefanzilske I saw #364 had been around for some time, we did not really plan to do this (especially the desire to just print "a" or something) until a much later version, but it seems fair to "hijack" it for your particular requirement and customize both toString() and parse(), e.g. by restricting the scale.

I linked them this way, but it would be nice to mention a few words about the requirement in #364 if you can.

Btw, there are currently no plans to configure Moneta via the SpringBoot application.properties. Although the Spring Framework (and thus also Spring Boot) support formatting via JSR 354, the JSR is runtime-agnostic and works with all other Microframeworks just as well.