Bug 57951 - DataFormatter.formatRawCellContents doesn't round properly with JDK8 due to not using BigDecimal
Summary: DataFormatter.formatRawCellContents doesn't round properly with JDK8 due to n...
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: SS Common (show other bugs)
Version: 3.12-FINAL
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-26 22:59 UTC by Robert Kish
Modified: 2015-05-27 17:15 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Kish 2015-05-26 22:59:00 UTC
JDK8 corrected some rounding issues with NumberFormat/DecimalFormat that effect some number formatting while processing Excel files. (See http://stackoverflow.com/questions/22797964/is-inconsistency-in-rounding-between-java-7-and-java-8-a-bug). These problems are noticed when not using BigDecimal.

If I add the Junit mod below to org.apache.poi.ss.usermodel.TestDataFormatter, it passes with JDK6, but fails with JDK8.

    public void testRounding() {
        DataFormatter dfUS = new DataFormatter(Locale.US);

        assertEquals("13.90", dfUS.formatRawCellContents(13.895, 2, "0.00"));
        assertEquals("13.91", dfUS.formatRawCellContents(13.905, 2, "0.00"));
        assertEquals("13.92", dfUS.formatRawCellContents(13.915, 2, "0.00"));
        assertEquals("13.93", dfUS.formatRawCellContents(13.925, 2, "0.00"));
        assertEquals("13.94", dfUS.formatRawCellContents(13.935, 2, "0.00"));
        assertEquals("13.95", dfUS.formatRawCellContents(13.945, 2, "0.00"));
        assertEquals("13.96", dfUS.formatRawCellContents(13.955, 2, "0.00"));
        assertEquals("13.97", dfUS.formatRawCellContents(13.965, 2, "0.00"));
        assertEquals("13.98", dfUS.formatRawCellContents(13.975, 2, "0.00"));
        assertEquals("13.99", dfUS.formatRawCellContents(13.985, 2, "0.00"));
        assertEquals("14.00", dfUS.formatRawCellContents(13.995, 2, "0.00"));
        assertEquals("14.01", dfUS.formatRawCellContents(14.005, 2, "0.00"));
     }

If you make the code change below to org.apache.poi.ss.usermodel.DataFormatter, the test above will pass for JDK6 and JDK8.

public String formatRawCellContents(double value, int formatIndex, String formatString, boolean use1904Windowing) {
     :
     :

     // else Number
         Format numberFormat = getFormat(value, formatIndex, formatString);
         if (numberFormat == null) {
             return String.valueOf(value);
         }
         
         String result;
         // When formatting 'value', double to text to BigDecimal produces more
         // accurate results than double to Double in JDK8 (as compared to
         // previous versions). However, if the value contains E notation, this
         // would expand the values, which we do not want, so revert to
         // original method.
         final String textValue = NumberToTextConverter.toText(value);
         if (textValue.indexOf('E') > -1) {
             result = numberFormat.format(new Double(value));
         }
         else {
             result = numberFormat.format(new BigDecimal(textValue));
         }
         // Complete scientific notation by adding the missing +.
         if (result.indexOf('E') > -1 && !result.contains("E-")) {
             result = result.replaceFirst("E", "E+");
         }
         return result;
}

I apologize for not taking the time to learn how to submit a proper patch, but I believe this code is all that is needed for this issue. Also, I changed some of the previous comments regarding the E notation (I was the RK in those comments).

Thanks
Comment 1 Nick Burch 2015-05-27 08:59:06 UTC
Any chance you could get a JDK9 snapshot, and check with that too? While we're doing Java version compatibility fixes, it would seem a good chance to do all of them at once!
Comment 2 Robert Kish 2015-05-27 16:03:14 UTC
Using build 1.9.0-ea-b65, I get the same behavior as in build 1.8.0_45-b14; my new Junit test fails with poi 3.12 code and it succeeds with modified code using BigDecimal.
Comment 3 Nick Burch 2015-05-27 17:15:41 UTC
Thanks for this investigation!

I've applied your change in r1682083.