Hi David. I have revised my fix for this CR. Here's a description of the most recent changes:

   1. The code fix has been moved slightly from it's original position
   into a section within FormattedFloatingDecimal.java which handles
   0.0* numbers.

   2. The code fix no longer changes the precision value.

   3. The code fix checks to see if the precision is 1 and the number
   is a one-digit 0.  If not, the normal code which adds the decimal
   and remaining digits is run.  If so, this block of code is not run.

In addition, I have re-run the Basic-X set of Formatter tests to make sure there aren't any regressions.

   Webrev: http://cr.openjdk.java.net/~bpassani/6469160/1/webrev/
   <http://cr.openjdk.java.net/%7Ebpassani/6469160/1/webrev/>


Thanks.

On 1/24/2012 11:58 PM, David Holmes wrote:
Hi Brandon,

On 25/01/2012 5:03 PM, Brandon Passanisi wrote:
Hi David. Thank you for your review comments. My answers to your
questions are below:

On 1/23/2012 9:56 PM, David Holmes wrote:
More generally it is not clear to me that putting in this special case
is the right way to fix this. Though I admit I don't really understand
what the specification requires when you give a precision of 0 with a
'g' conversion:

"If the conversion is 'g' or 'G', then the precision is the total
number of digits in the resulting magnitude after rounding."

So we asked for zero digits? What should that mean?

The Formatter javadoc, within the "Float and Double" section, describes
the following regarding a value of 0 for precision and the 'g'/'G'
conversion [1]:

"If the precision is 0, then it is taken to be 1"

Ah! Thanks. I hadn't seen that (wish they wouldn't split up the spec for this across different sections!).

Okay that explains the 0/1 situation.

But that makes me question the "rightness" of the fix even more. We took steps to change a precision of 0 to 1, but then the fix changes it back to 0 because otherwise something else breaks. Seems to me that the "something else" that handles the precision of 1 incorrectly is the code that really needs to be fixed. Further it suggest that there may be assumptions in later code that precision is in fact never 0.

David
-----


The following code block within Formatter.java near line 3278 is run to
do this:

if (precision == -1)
prec = 6;
else if (precision == 0)
prec = 1;

And the precision value "prec" is given to FormattedFloatingDecimal.
Therefore, when this particular error condition is seen and the proposed
code fix is reached in FormattedFloatingDecimal.java, the precision will
be 1. So, the proposed code fix ends up supporting a precision value of
0 and 1.


[1]: http://docs.oracle.com/javase/7/docs/api/java/util/Formatter.html


Thanks.


David
-----

Since java has been throwing exceptions in these cases, I consulted with the output of C's printf to make sure that the outputted strings are the same. I updated the Formatter's Basic-X template of tests with a little
over 20 test format strings that were causing exceptions without the
change and the output of each is compared with the output from C's
printf with the same format string. And, I ran all of the Basic-X tests
to make sure there weren't any regressions in behavior.

Thanks.


--
Oracle <http://www.oracle.com>
Brandon Passanisi | Principle Member of Technical Staff


Green Oracle <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment

Reply via email to