Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits #3363

Closed
wants to merge 3 commits into from
Closed

8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits #3363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ec00f8
Fixing rounding issue with BigDecimal and %g formatting flags
igraves Apr 6, 2021
cf40421
Adding BigDecimal equivalents to tests
igraves Apr 7, 2021
4552260
Inlining some single use variables
igraves Apr 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/java.base/share/classes/java/util/Formatter.java
View file
Open in desktop
Expand Up @@ -3821,11 +3821,10 @@ private void print(StringBuilder sb, BigDecimal value, Locale l,
else if (precision == 0)
prec = 1;

BigDecimal tenToTheNegFour = BigDecimal.valueOf(1, 4);
BigDecimal tenToThePrec = BigDecimal.valueOf(1, -prec);
value = value.round(new MathContext(prec));
igraves marked this conversation as resolved.
Show resolved Hide resolved
if ((value.equals(BigDecimal.ZERO))
|| ((value.compareTo(tenToTheNegFour) != -1)
&& (value.compareTo(tenToThePrec) == -1))) {
|| ((value.compareTo(BigDecimal.valueOf(1, 4)) != -1)
&& (value.compareTo(BigDecimal.valueOf(1, -prec)) == -1))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that compareTo in general specifies a negative, zero, or positive return value, but BigDecimal and BigInteger specify a return value of -1, 0, and 1. So the code here that compares against -1 is strictly correct. However, the BigDecimal/BigInteger.compareTo docs say "The suggested idiom..." is a relative comparison against zero.

Indeed, the BigDecimal::compareTo method does always seem to return -1, 0, or 1 so this code is not incorrect. Well, maybe. I checked quickly and the BigDecimal comparison logic is fairly intricate (and also runs through BigInteger) so I might have missed something. Also, BigDecimal is subclassable, so an override of compareTo might return something other than -1, 0, or 1, even though strictly speaking this would violate the BigDecimal spec.

I'm wondering if there should be a followup bug that changes these tests to >= 0 and < 0.


int e = - value.scale()
+ (value.unscaledValue().toString().length() - 1);
Expand Down
62 changes: 62 additions & 0 deletions test/jdk/java/util/Formatter/BigDecimalRounding.java
View file
Open in desktop
@@ -0,0 +1,62 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/**
* @test
* @bug 8262744
* @summary BigDecimal does not always display formatting correctly because
* rounding is done after formatting check. Fix moves rounding to before the
* range-based formatting check for %g formatting flag.
* @run testng BigDecimalRounding
*/

import org.testng.annotations.Test;

import java.math.BigDecimal;

import static org.testng.Assert.*;

@Test
public class BigDecimalRounding {

public static void testBigDecimalRounding() {
var res1 = String.format("%g", 0.00009999999999999995);
var res2 = String.format("%g", 0.00009999999f);
igraves marked this conversation as resolved.
Show resolved Hide resolved
var res3 = String.format("%g", new BigDecimal(0.0001));
var res4 = String.format("%g", new BigDecimal("0.00009999999999999999995"));

assertEquals(res1, res2);
assertEquals(res2, res3);
assertEquals(res3, res4);

var res5 = String.format("%.9g", 999999.999999432168754e+3);
var res6 = String.format("%.9g", 999999999.999432168754f);
var res7 = String.format("%.9g", new BigDecimal("999999.999999432168754e+3")); // !!
var res8 = String.format("%.9g", new BigDecimal("1000000000")); // !!

assertEquals(res5, res6);
assertEquals(res6, res7);
assertEquals(res7, res8);

}
}