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.lagn...@oracle.com
<mailto:olivier.lagn...@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
