United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-7076409 : DecimalFormat.format() truncating output

Details
Type:
Bug
Submit Date:
2011-08-08
Status:
Closed
Updated Date:
2012-03-20
Project Name:
JDK
Resolved Date:
2011-12-21
Component:
core-libs
OS:
windows_vista
Sub-Component:
java.text
CPU:
x86
Priority:
P4
Resolution:
Duplicate
Affected Versions:
6u26
Fixed Versions:

Related Reports
Duplicate:

Sub Tasks

Description
FULL PRODUCT VERSION :
java version "1.6.0_24"
Java(TM) SE Runtime Environment (build 1.6.0_24-b07)
Java HotSpot(TM) 64-Bit Server VM (build 19.1-b02, mixed mode)

ADDITIONAL OS VERSION INFORMATION :
Microsoft Windows [Version 6.1.7601]
Also verified on Mac OS X 10.6.8

A DESCRIPTION OF THE PROBLEM :
The DecimalFormat class returns inconsistent values when calling the format(double number) method.

According to the documentation for DecimalFormat: "#,##0.0#;(#)" produces precisely the same behavior as "#,##0.0#;(#,##0.0#)"

However, this is not the case. The output produced by the former truncates the closing parenthesis for negative values. The latter appears to work correctly.

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Run the following JUnit test method to reproduce the problem:

@Test
    public void testDecimalFormat() {
        double value = -4000d;

        final String expected = "(4,000.00)";
        final String actualA = new DecimalFormat("#,##0.00;(#,##0.00)").format(value);
        final String actualB = new DecimalFormat("#,##0.00;(#)").format(value);

        assertEquals(expected, actualA);
        assertEquals(expected, actualB);
    }

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
Both assertions should succeed, as the output should be identical as per the documentation.
ACTUAL -
The assertion on actualB fails as the trailing parenthesis is missing:

Expected :(4,000.00)
Actual   :(4,000.00

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
package formatting;

import org.junit.Test;

import java.text.DecimalFormat;

import static org.junit.Assert.assertEquals;

public class FormatTest {

    @Test
    public void testDecimalFormat() {
        double value = -4000d;

        final String expected = "(4,000.00)";
        final String actualA = new DecimalFormat("#,##0.00;(#,##0.00)").format(value);
        final String actualB = new DecimalFormat("#,##0.00;(#)").format(value);

        assertEquals(expected, actualA);
        assertEquals(expected, actualB);
    }
}

---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
The format must be fully specified (i.e. duplicated) for both the positive and negative patterns in order for this to work until a bug fix is released.

                                    

Comments
EVALUATION

Closing this bug as a duplicate of 6609740.
                                     
2011-12-21
PUBLIC COMMENTS

From: https://bugs.openjdk.java.net/show_bug.cgi?id=100219

Comment #3 From A.Gueganno 2011-12-16 05:35:41 PDT (-) [reply]

Nice to see bugs for Java 6 are still considered active for openJDK 7 ;-)

I've filled the bug here, and am still replying to it, because I was wondering
if this bug could still be fixed for java 6. As openJDK 7 is younger (well
done, captain obvious :-p ), I thought it may have a better chance to be fixed
here. And in order to help people to backport the potential fix, I gave the
link to the original bug. If you think my comments should be placed in the Java
6 tracker, I'm ready to post them there ;-)

I think there are many incoherences between the internal documentation of
DecimalFormat and its implementation. Moreover, as pointed before, The
implementation is unable to follow the specs of the class.

I think the problem comes from the strategy used to process the patterns for
negative numbers. Indeed, if we have a look in DecimalFormat, we can find (line
2491) : 

case 1:
   // Phase one must be identical in the two sub-patterns. We
   // enforce this by doing a direct comparison. While
   // processing the first sub-pattern, we just record its
   // length. While processing the second, we compare
   // characters.
   if (j == 1) {
      ++phaseOneLength;
   } else {
      if (--phaseOneLength == 0) {
         phase = 2;
         affix = suffix;
      }
      continue;
   }

If we follow the javadoc, the negative pattern should just be ignored.
According to the inner documentation, we are supposed to check the two patterns
are identical. Actually, we are just comparing their lengths...

Here, if j==1, we are analyzing the positive pattern. Otherwise, it is the
negative one. If we are in the negative pattern, we continue our work until we
have treated enough characters. Then we extract the negative suffixe.

What we could in order to obtain the expected behaviour is to check that the
character at the current position can be used in a pattern. If it is, we
continue. If it is not, we decrement the int that is recording our current
position, set phase to 2, and continue.

Considering this, we finally have two strategies. First we can decide we don't
care that the negative pattern is valid. Our code becomes :

case 1:
   if (j < 1)  {
      if (!(ch == digit || ch == zeroDigit  
              || ch == groupingSeparator || ch == decimalSeparator 
              || pattern.regionMatches(pos, exponent, 
                                  0, exponent.length()))) {
         phase = 2;
         affix = suffix;
         --p;
      }
      continue;
   }

In this code, we test we're in the negative pattern. If so, we just read the
character (int the outer for loop) and test if it is usable in a pattern. If it
is, we go to the next character (with the continue instruction). If it is not,
we set affix to suffix (we've finished to read the pattern), we go in phase 2
(we read the suffix), and we decrement p. Indeed, it's part of the suffix, so
we have to include it ;-).


If the validity of the pattern still matters, we can still use the code used
for the 

case 1:
   if (j < 1)  {
      if (!(ch == digit || ch == zeroDigit  
              || ch == groupingSeparator || ch == decimalSeparator 
              || pattern.regionMatches(pos, exponent, 
                                  0, exponent.length()))) {
         phase = 2;
         affix = suffix;
         --p;
         continue;
      }
   }

The only difference is the place of the continue instruction. We go back to the
beginning of the for loop if and only if we have encountered an invalid
character. If the current character is valid, we continue our treatment,
building a ghost pattern. If we are dealing with an invalid pattern, an
exception will be thrown.

Note that this second approach will force some refactoring in this part of the
method. Indeed, we'll have to store useExponentialNotation and
minExponentDigits in local variables, and affect them to the class attributes
only when j == 1.

I know this is not really a patch... And actually, I could have missed
something. I do hope it will be useful anyway, and maybe quickly usable ;-)
                                     
2011-12-18



Hardware and Software, Engineered to Work Together