Urgent [9] RFR (S) : JDK-8039915 NumberFormat.format() does not consider required no. of fraction digits properly

[olivier.lagneau at oracle.com]

Thanks Brian,

Will take into account  your  wording remarks.

Olivier.
On 22/09/2014 17:50, Brian Burkhalter wrote:
> Hi Olivier,
>
> On Sep 22, 2014, at 7:56 AM, olivier.lagneau at oracle.com 
> <mailto:olivier.lagneau at oracle.com> wrote:
>
>>> and 2) use braces around all the statements contained in if/else 
>>> blocks (see below). Comment #2 is nit-picky.
>> I tried to keep the same flavour of writing as in HALF_DOWN and 
>> HALF_EVEN cases, i.e. don't use brace for
>> terminal/leaf return true/false statements. This is not the standard 
>> however, at least in this file.
>> Will use braces in all case (i.e. the 3 of HALF_UP,  HALF_DOWN and 
>> HALF_EVEN).
>
> I did not look at the other case. If your formatting matches the rest 
> of the file then I think it is OK to leave it as-is.
>
>>> Lastly and this is not really part of your changeset, but I found it 
>>> curious that the test prints the details of all cases that succeed 
>>> as opposed to those that fail. I would think that either all results 
>>> or the failures alone ought to be printed instead of successes only. 
>>> See for example the partial diff below the DigitList diff.
>> Since these are most often corner and tricky test cases I think it 
>> interesting to have the details of each result,
>> including infos of both why returned result is correct or wrong.
>> That can help the reader to understand all these tricky cases.
>> The bad side of it being that it prints a lot of text, with failure 
>> cases (hoepfully few) lost in the middle of it,
>> thus making failures possibly not immediate to detect.
>>
>> Here is an example of what is printed in case of failure:
>> "
>> ========================================
>> ***Error formatting double value from string : 0.6868d
>> NumberFormat pattern is  : #,##0.###
>> Maximum number of fractional digits : 3
>> Fractional rounding digit : 4
>> Position of value relative to tie : above
>> Rounding Mode : HALF_UP
>> BigDecimal output : 
>> 0.68679999999999996607158436745521612465381622314453125
>> FloatingDecimal output : 0.6868
>> Error. Formatted result different from expected.
>> Expected output is : "0.687"
>> Formated output is : "0.686"
>> ========================================
>
> I missed that output: I was looking for the word “failure.”
>
>> There is also a reminder of the number of errors at the end of the 
>> report:
>> "
>> ==> 4 tests failed
>>
>> Test failed with 4 formating error(s).
>> "
>>
>> May be providing a reminder (value + rounding-mode + rounding position)
>> of the failure cases at the end of the would be better ?
>> Like:
>> "
>> Test failed with 4 formating error(s) for following cases :
>> - 0.3126d, HALF_UP rounding, 3 fractional digits
>> - 0.6868d, HALF_UP rounding, 3 fractional digits
>> - 1.798876d, HALF_UP rounding, 5 fractional digits
>> - 1.796889d, HALF_UP rounding, 5 fractional digits
>> "
>>
>> Would doing so be ok ?
>
> If the test is already printing out the information you showed above 
> (“Error formatting …”) then I think it is enough but the verbiage 
> should perhaps match the reminder, e.g., “Failure: Error formatting 
> double …”
>
> Thanks,
>
> Brian