Apply default rounding on MonetaryAmount.ToString

[atsticks]

When using toString() on an amount in Moneta it returns the full number, which is not what most users would expect. Instead I propose to apply default rounding before creating the amount String returned, e.g. the following snippet

Money money = Monetary.getAmountFactory(Money.class).setCurrency("CHF").setNumber(2000.12345).create();
System.err.println(money);

results in CHF 2000.12 instead of CHF 2000.12345. I think this is what most users would expect.

[atsticks]

Code fix for money:

public String toString() {
        try {
            MonetaryAmount amount = Monetary.getDefaultRounding().apply(this);
            return getCurrency().getCurrencyCode() + ' ' + 
                    amount.getNumber().numberValue(BigDecimal.class).toPlainString();
        }catch(Exception e) {
            return getCurrency().getCurrencyCode() + ' ' + number.toPlainString();
        }
}

[stokito]

Just to clarify: the number will be rounded to the currency's fractional digits?
And IMHO we need to keep the ability to format string with full number.
Another problem that some code may relate on the old behavior and there may be very tricky situation in production code. So the change should be flowed with increasing a minor version.
Or we can create a separate method with the new behavior but IMHO it's better to change toString() itself because it is used anywhere even implicitly like in log statements.

[stokito]

Thank you for the change but I'm afraid we have to rollback it.
First of all the change was committed in 7926947 with a comment that have no relation to the task. It looks like it was committed accidentally.

So please rollback and recommit it with the ticket number and then force push.
Also it would be nice to create a PR and review it before the push to master. At least to avoid some small mistakes.
Also it looks like the change should be applied to FastMoney.
UPD I pulled changes and checked git log and now I see that there was another commits but they was committed under #275 ticket which is absolutely have no any relation to this. I still think it would be better to recommit it with cleared changes and comments.

Speaking about the change itself: it wasn't properly discussed, reviewed or tested while it affects backward compatibility. Only I left a comment with some concern about it and even the comment wasn't answered. Let me extend it.

I'm pretty sure that we shouldn't do this at least in this manner.
The reasons are:

0. The change breaking behavior backward compatibility but users may not even note that change while they should change their codebase and retest it.

1. The change breaking behavior backward compatibility and a lot of users will note that change. So eventually almost all of them will be confused and they'll start to search for an answer on StackOverflow. Yep, nobody will read changelog for some library that was added years ago and now (often accidentally by transitive dependencies) updated. This is not a patch or minor, this a mayor change (i.e. for JavaMony v2.0.0). Also this definitely should be added to specification and all JSR354 implementors should do the same.

2. It significantly decreases performance especially in terms of the memory usage and this can lead even to OutOfMemory errors on production. Imagine that you have a heavy calculations or an export to CSV process that was already tested but now it will eat much more memory. I had such situation in practice (not with JavaMoney, but with another library) and that was horrible.

3. The change breaks some unsound value types contract: each value type (like integer or date) can be serialized to string with toString() call and then safely desiarilized back without losing of data.

For example when you do the Integer.toString() then it always use the same format with dot's. If you want to format is with comas or to locale format you have to use the NumberFormat.
But instead you can always to new Integer(intAsStr) and be sure that you'll get exactly the same number.
The contract is broken for java.util.Date and this caused a lot of problems. That's why the new JavaTime API always have a fixed serialization format in toString().

If the Money.toString() produces an output that is not rounded to fraction digits then this is ok because instead you can deserialize the string back and receive the same precise amount. Maybe that 0.000001 during calculations will be important if you have a lot of them.

5. There is reason to use toString() because it always gives exactly the same format. I.e. when users wants the opposite behavior that it have now. For example in tests.
   Now all tests become locale dependent and they will fail. For example on my computer during mvn clean install I got this:

Results :
Failed tests: 
  DefaultMonetaryAmountFormatTest.testParse:70 expected [USD 1'000.42] but found [USD1,000.42]
  DefaultMonetaryAmountFormatTest.testParse_pattern_without_currency_sign_but_with_currency_in_context:110 expected [USD 0.01] but found [USD0.01]
  DefaultMonetaryAmountFormatTest.testParse_with_custom_pattern:82 expected [USD 0.01] but found [USD0.01]
  MonetaryFormatsTest.testFormat_BGN_bg_BG:100->assertMoneyFormat:198 expected [14 000,12 BGN] but found [14000,12 BGN]
  MonetaryFormatsTest.testParse_BGN_bg_BG:91->assertMoneyParse:191 » IllegalArgument
  MonetaryAmountFormatTest.testBulgarianLev:220 expected [1 123 000,50 лв.] but found [1123000,50 лв.]
  ToStringMonetaryAmountFormatTest.parserFastMoneyTest:93->executeTest:188->parser:198 » MonetaryParse
  ToStringMonetaryAmountFormatTest.parserMoneyTest:87->executeTest:188->parser:198 » MonetaryParse
  ToStringMonetaryAmountFormatTest.parserRoundedMoneyTest:99->executeTest:188->parser:198 » MonetaryParse
  ToStringMonetaryAmountFormatTest.shoudReturnToStringOnPrintWhenHasMonetaryWithFastMoney:153 expected [BRL 10.00] but found [BRL10.00]
  ToStringMonetaryAmountFormatTest.shoudReturnToStringOnPrintWhenHasMonetaryWithMoney:168 expected [BRL 10.00] but found [BRL10.00]
  ToStringMonetaryAmountFormatTest.shoudReturnToStringOnPrintWhenHasMonetaryWithRoundedMoney:182 expected [BRL 10.00] but found [BRL10.00]
  ToStringMonetaryAmountFormatTest.shoudReturnToStringOnQueryFromWhenMonetaryWithFastMoney:113 expected [BRL 10.00] but found [BRL10.00]
  ToStringMonetaryAmountFormatTest.shoudReturnToStringOnQueryFromWhenMonetaryWithMoney:126 expected [BRL 10.00] but found [BRL10.00]
  ToStringMonetaryAmountFormatTest.shoudReturnToStringOnQueryFromWhenMonetaryWithRoundedMoney:139 expected [BRL 10.00] but found [BRL10.00]
Tests run: 581, Failures: 15, Errors: 0, Skipped: 0

This means that the master branch is broken.

6. Here is mixed two things: rounding to the currencies fraction digits and localizing the amount. In fact this is a two separate things. For example I wan't to have an ability to always format numbers with a dot but rounded to default currency digits i.e. EUR 12.34 while the real number is EUR 12.345678 no matter what is my locale.

So here I have a proposition:
Let's rollback the change. And then we can create a separate method toLocaleString() or getDisplayString(). Or maybe we can just keep it as is i.e. users needs to call the MonetryFormatter themselves. Anyway we should add a note to the toString() method that it produces not a human readable string.

[stokito]

friendly reminder @atsticks

[odrotbohm]

This is a problematic change, esp. as it breaks assertion error messages as they now effectively report an invalid value (a formatted one instead of the real one. Same applies to values shown in a debugger.

I've filed #357 to hopefully roll this back. I'd argue that toString() needs to be a precise representation of the instance's state. If you want formatting, use the formatters. We need to be able to serialize the exact instance state.