From a1cc16ccd3f05834c961f070d205272a5c45904a Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Wed, 24 Oct 2018 21:25:39 -0700 Subject: [PATCH] ICU-13701 Refactoring DecimalQuantity: removing lOptPos/rOptPos. Combined ICU4C and ICU4J. --- icu4c/source/i18n/number_decimalquantity.cpp | 62 ++++++++------ icu4c/source/i18n/number_decimalquantity.h | 63 +++++++------- icu4c/source/i18n/number_integerwidth.cpp | 5 +- icu4c/source/i18n/number_rounding.cpp | 18 ++-- .../intltest/numbertest_decimalquantity.cpp | 80 ++++++++++-------- .../ibm/icu/impl/number/DecimalQuantity.java | 21 +++-- .../number/DecimalQuantity_AbstractBCD.java | 84 ++++++++----------- .../DecimalQuantity_DualStorageBCD.java | 17 +++- .../ibm/icu/number/NumberFormatterImpl.java | 10 ++- .../src/com/ibm/icu/number/Precision.java | 16 ++-- .../impl/number/DecimalQuantity_64BitBCD.java | 10 ++- .../number/DecimalQuantity_ByteArrayBCD.java | 13 ++- .../number/DecimalQuantity_SimpleStorage.java | 34 ++++---- .../dev/test/number/DecimalQuantityTest.java | 46 ++++++---- 14 files changed, 261 insertions(+), 218 deletions(-) diff --git a/icu4c/source/i18n/number_decimalquantity.cpp b/icu4c/source/i18n/number_decimalquantity.cpp index d5dd7ae694c..5a1fc331206 100644 --- a/icu4c/source/i18n/number_decimalquantity.cpp +++ b/icu4c/source/i18n/number_decimalquantity.cpp @@ -112,10 +112,8 @@ DecimalQuantity& DecimalQuantity::operator=(DecimalQuantity&& src) U_NOEXCEPT { void DecimalQuantity::copyFieldsFrom(const DecimalQuantity& other) { bogus = other.bogus; - lOptPos = other.lOptPos; lReqPos = other.lReqPos; rReqPos = other.rReqPos; - rOptPos = other.rOptPos; scale = other.scale; precision = other.precision; flags = other.flags; @@ -125,18 +123,15 @@ void DecimalQuantity::copyFieldsFrom(const DecimalQuantity& other) { } void DecimalQuantity::clear() { - lOptPos = INT32_MAX; lReqPos = 0; rReqPos = 0; - rOptPos = INT32_MIN; flags = 0; setBcdToZero(); // sets scale, precision, hasDouble, origDouble, origDelta, and BCD data } -void DecimalQuantity::setIntegerLength(int32_t minInt, int32_t maxInt) { +void DecimalQuantity::setMinInteger(int32_t minInt) { // Validation should happen outside of DecimalQuantity, e.g., in the Precision class. U_ASSERT(minInt >= 0); - U_ASSERT(maxInt >= minInt); // Special behavior: do not set minInt to be less than what is already set. // This is so significant digits rounding can set the integer length. @@ -145,28 +140,37 @@ void DecimalQuantity::setIntegerLength(int32_t minInt, int32_t maxInt) { } // Save values into internal state - // Negation is safe for minFrac/maxFrac because -Integer.MAX_VALUE > Integer.MIN_VALUE - lOptPos = maxInt; lReqPos = minInt; } -void DecimalQuantity::setFractionLength(int32_t minFrac, int32_t maxFrac) { +void DecimalQuantity::setMinFraction(int32_t minFrac) { // Validation should happen outside of DecimalQuantity, e.g., in the Precision class. U_ASSERT(minFrac >= 0); - U_ASSERT(maxFrac >= minFrac); // Save values into internal state // Negation is safe for minFrac/maxFrac because -Integer.MAX_VALUE > Integer.MIN_VALUE rReqPos = -minFrac; - rOptPos = -maxFrac; +} + +void DecimalQuantity::applyMaxInteger(int32_t maxInt) { + // Validation should happen outside of DecimalQuantity, e.g., in the Precision class. + U_ASSERT(maxInt >= 0); + + if (precision == 0) { + return; + } + + int32_t magnitude = getMagnitude(); + if (maxInt <= magnitude) { + popFromLeft(magnitude - maxInt + 1); + compact(); + } } uint64_t DecimalQuantity::getPositionFingerprint() const { uint64_t fingerprint = 0; - fingerprint ^= lOptPos; fingerprint ^= (lReqPos << 16); fingerprint ^= (static_cast(rReqPos) << 32); - fingerprint ^= (static_cast(rOptPos) << 48); return fingerprint; } @@ -280,7 +284,7 @@ int32_t DecimalQuantity::getUpperDisplayMagnitude() const { U_ASSERT(!isApproximate); int32_t magnitude = scale + precision; - int32_t result = (lReqPos > magnitude) ? lReqPos : (lOptPos < magnitude) ? lOptPos : magnitude; + int32_t result = (lReqPos > magnitude) ? lReqPos : magnitude; return result - 1; } @@ -290,7 +294,7 @@ int32_t DecimalQuantity::getLowerDisplayMagnitude() const { U_ASSERT(!isApproximate); int32_t magnitude = scale; - int32_t result = (rReqPos < magnitude) ? rReqPos : (rOptPos > magnitude) ? rOptPos : magnitude; + int32_t result = (rReqPos < magnitude) ? rReqPos : magnitude; return result; } @@ -511,7 +515,7 @@ int64_t DecimalQuantity::toLong(bool truncateIfOverflow) const { // if (dq.fitsInLong()) { /* use dq.toLong() */ } else { /* use some fallback */ } // Fallback behavior upon truncateIfOverflow is to truncate at 17 digits. uint64_t result = 0L; - int32_t upperMagnitude = std::min(scale + precision, lOptPos) - 1; + int32_t upperMagnitude = scale + precision - 1; if (truncateIfOverflow) { upperMagnitude = std::min(upperMagnitude, 17); } @@ -527,7 +531,7 @@ int64_t DecimalQuantity::toLong(bool truncateIfOverflow) const { uint64_t DecimalQuantity::toFractionLong(bool includeTrailingZeros) const { uint64_t result = 0L; int32_t magnitude = -1; - int32_t lowerMagnitude = std::max(scale, rOptPos); + int32_t lowerMagnitude = scale; if (includeTrailingZeros) { lowerMagnitude = std::min(lowerMagnitude, rReqPos); } @@ -884,10 +888,8 @@ UnicodeString DecimalQuantity::toScientificString() const { result.append(u"0E+0", -1); return result; } - // NOTE: It is not safe to add to lOptPos (aka maxInt) or subtract from - // rOptPos (aka -maxFrac) due to overflow. - int32_t upperPos = std::min(precision + scale, lOptPos) - scale - 1; - int32_t lowerPos = std::max(scale, rOptPos) - scale; + int32_t upperPos = precision - 1; + int32_t lowerPos = 0; int32_t p = upperPos; result.append(u'0' + getDigitPos(p)); if ((--p) >= lowerPos) { @@ -985,6 +987,18 @@ void DecimalQuantity::shiftRight(int32_t numDigits) { precision -= numDigits; } +void DecimalQuantity::popFromLeft(int32_t numDigits) { + if (usingBytes) { + int i = precision - 1; + for (; i >= precision - numDigits; i--) { + fBCD.bcdBytes.ptr[i] = 0; + } + } else { + fBCD.bcdLong &= (static_cast(1) << ((precision - numDigits) * 4)) - 1; + } + precision -= numDigits; +} + void DecimalQuantity::setBcdToZero() { if (usingBytes) { uprv_free(fBCD.bcdBytes.ptr); @@ -1239,10 +1253,8 @@ bool DecimalQuantity::operator==(const DecimalQuantity& other) const { scale == other.scale && precision == other.precision && flags == other.flags - && lOptPos == other.lOptPos && lReqPos == other.lReqPos && rReqPos == other.rReqPos - && rOptPos == other.rOptPos && isApproximate == other.isApproximate; if (!basicEquals) { return false; @@ -1272,11 +1284,9 @@ UnicodeString DecimalQuantity::toString() const { snprintf( buffer8, sizeof(buffer8), - "", - (lOptPos > 999 ? 999 : lOptPos), + "", lReqPos, rReqPos, - (rOptPos < -999 ? -999 : rOptPos), (usingBytes ? "bytes" : "long"), (isNegative() ? "-" : ""), (precision == 0 ? "0" : digits.getAlias()), diff --git a/icu4c/source/i18n/number_decimalquantity.h b/icu4c/source/i18n/number_decimalquantity.h index 2bef2514bce..af00917e2b9 100644 --- a/icu4c/source/i18n/number_decimalquantity.h +++ b/icu4c/source/i18n/number_decimalquantity.h @@ -53,22 +53,28 @@ class U_I18N_API DecimalQuantity : public IFixedDecimal, public UMemory { DecimalQuantity &operator=(DecimalQuantity&& src) U_NOEXCEPT; /** - * Sets the minimum and maximum integer digits that this {@link DecimalQuantity} should generate. + * Sets the minimum integer digits that this {@link DecimalQuantity} should generate. * This method does not perform rounding. * * @param minInt The minimum number of integer digits. - * @param maxInt The maximum number of integer digits. */ - void setIntegerLength(int32_t minInt, int32_t maxInt); + void setMinInteger(int32_t minInt); /** - * Sets the minimum and maximum fraction digits that this {@link DecimalQuantity} should generate. + * Sets the minimum fraction digits that this {@link DecimalQuantity} should generate. * This method does not perform rounding. * * @param minFrac The minimum number of fraction digits. - * @param maxFrac The maximum number of fraction digits. */ - void setFractionLength(int32_t minFrac, int32_t maxFrac); + void setMinFraction(int32_t minFrac); + + /** + * Truncates digits from the upper magnitude of the number in order to satisfy the + * specified maximum number of integer digits. + * + * @param maxInt The maximum number of integer digits. + */ + void applyMaxInteger(int32_t maxInt); /** * Rounds the number to a specified interval, such as 0.05. @@ -336,36 +342,11 @@ class U_I18N_API DecimalQuantity : public IFixedDecimal, public UMemory { */ int32_t origDelta; - // Four positions: left optional '(', left required '[', right required ']', right optional ')'. - // These four positions determine which digits are displayed in the output string. They do NOT - // affect rounding. These positions are internal-only and can be specified only by the public - // endpoints like setFractionLength, setIntegerLength, and setSignificantDigits, among others. - // - // * Digits between lReqPos and rReqPos are in the "required zone" and are always displayed. - // * Digits between lOptPos and rOptPos but outside the required zone are in the "optional zone" - // and are displayed unless they are trailing off the left or right edge of the number and - // have a numerical value of zero. In order to be "trailing", the digits need to be beyond - // the decimal point in their respective directions. - // * Digits outside of the "optional zone" are never displayed. - // - // See the table below for illustrative examples. - // - // +---------+---------+---------+---------+------------+------------------------+--------------+ - // | lOptPos | lReqPos | rReqPos | rOptPos | number | positions | en-US string | - // +---------+---------+---------+---------+------------+------------------------+--------------+ - // | 5 | 2 | -1 | -5 | 1234.567 | ( 12[34.5]67 ) | 1,234.567 | - // | 3 | 2 | -1 | -5 | 1234.567 | 1(2[34.5]67 ) | 234.567 | - // | 3 | 2 | -1 | -2 | 1234.567 | 1(2[34.5]6)7 | 234.56 | - // | 6 | 4 | 2 | -5 | 123456789. | 123(45[67]89. ) | 456,789. | - // | 6 | 4 | 2 | 1 | 123456789. | 123(45[67]8)9. | 456,780. | - // | -1 | -1 | -3 | -4 | 0.123456 | 0.1([23]4)56 | .0234 | - // | 6 | 4 | -2 | -2 | 12.3 | ( [ 12.3 ]) | 0012.30 | - // +---------+---------+---------+---------+------------+------------------------+--------------+ - // - int32_t lOptPos = INT32_MAX; + // Positions to keep track of leading and trailing zeros. + // lReqPos is the magnitude of the first required leading zero. + // rReqPos is the magnitude of the last required trailing zero. int32_t lReqPos = 0; int32_t rReqPos = 0; - int32_t rOptPos = INT32_MIN; /** * The BCD of the 16 digits of the number represented by this object. Every 4 bits of the long map @@ -421,8 +402,22 @@ class U_I18N_API DecimalQuantity : public IFixedDecimal, public UMemory { */ void shiftLeft(int32_t numDigits); + /** + * Directly removes digits from the end of the BCD list. + * Updates the scale and precision. + * + * CAUTION: it is the caller's responsibility to call {@link #compact} after this method. + */ void shiftRight(int32_t numDigits); + /** + * Directly removes digits from the front of the BCD list. + * Updates precision. + * + * CAUTION: it is the caller's responsibility to call {@link #compact} after this method. + */ + void popFromLeft(int32_t numDigits); + /** * Sets the internal representation to zero. Clears any values stored in scale, precision, * hasDouble, origDouble, origDelta, and BCD data. diff --git a/icu4c/source/i18n/number_integerwidth.cpp b/icu4c/source/i18n/number_integerwidth.cpp index 6416b292982..d62aef444dc 100644 --- a/icu4c/source/i18n/number_integerwidth.cpp +++ b/icu4c/source/i18n/number_integerwidth.cpp @@ -43,14 +43,15 @@ void IntegerWidth::apply(impl::DecimalQuantity& quantity, UErrorCode& status) co if (fHasError) { status = U_ILLEGAL_ARGUMENT_ERROR; } else if (fUnion.minMaxInt.fMaxInt == -1) { - quantity.setIntegerLength(fUnion.minMaxInt.fMinInt, INT32_MAX); + quantity.setMinInteger(fUnion.minMaxInt.fMinInt); } else { // Enforce the backwards-compatibility feature "FormatFailIfMoreThanMaxDigits" if (fUnion.minMaxInt.fFormatFailIfMoreThanMaxDigits && fUnion.minMaxInt.fMaxInt < quantity.getMagnitude()) { status = U_ILLEGAL_ARGUMENT_ERROR; } - quantity.setIntegerLength(fUnion.minMaxInt.fMinInt, fUnion.minMaxInt.fMaxInt); + quantity.setMinInteger(fUnion.minMaxInt.fMinInt); + quantity.applyMaxInteger(fUnion.minMaxInt.fMaxInt); } } diff --git a/icu4c/source/i18n/number_rounding.cpp b/icu4c/source/i18n/number_rounding.cpp index ae4b8849fbe..44ea800ce67 100644 --- a/icu4c/source/i18n/number_rounding.cpp +++ b/icu4c/source/i18n/number_rounding.cpp @@ -348,9 +348,8 @@ void RoundingImpl::apply(impl::DecimalQuantity &value, UErrorCode& status) const getRoundingMagnitudeFraction(fPrecision.fUnion.fracSig.fMaxFrac), fRoundingMode, status); - value.setFractionLength( - uprv_max(0, -getDisplayMagnitudeFraction(fPrecision.fUnion.fracSig.fMinFrac)), - INT32_MAX); + value.setMinFraction( + uprv_max(0, -getDisplayMagnitudeFraction(fPrecision.fUnion.fracSig.fMinFrac))); break; case Precision::RND_SIGNIFICANT: @@ -358,12 +357,11 @@ void RoundingImpl::apply(impl::DecimalQuantity &value, UErrorCode& status) const getRoundingMagnitudeSignificant(value, fPrecision.fUnion.fracSig.fMaxSig), fRoundingMode, status); - value.setFractionLength( - uprv_max(0, -getDisplayMagnitudeSignificant(value, fPrecision.fUnion.fracSig.fMinSig)), - INT32_MAX); + value.setMinFraction( + uprv_max(0, -getDisplayMagnitudeSignificant(value, fPrecision.fUnion.fracSig.fMinSig))); // Make sure that digits are displayed on zero. if (value.isZero() && fPrecision.fUnion.fracSig.fMinSig > 0) { - value.setIntegerLength(1, INT32_MAX); + value.setMinInteger(1); } break; @@ -384,7 +382,7 @@ void RoundingImpl::apply(impl::DecimalQuantity &value, UErrorCode& status) const roundingMag = uprv_min(roundingMag, candidate); } value.roundToMagnitude(roundingMag, fRoundingMode, status); - value.setFractionLength(uprv_max(0, -displayMag), INT32_MAX); + value.setMinFraction(uprv_max(0, -displayMag)); break; } @@ -394,7 +392,7 @@ void RoundingImpl::apply(impl::DecimalQuantity &value, UErrorCode& status) const fRoundingMode, fPrecision.fUnion.increment.fMaxFrac, status); - value.setFractionLength(fPrecision.fUnion.increment.fMinFrac, INT32_MAX); + value.setMinFraction(fPrecision.fUnion.increment.fMinFrac); break; case Precision::RND_CURRENCY: @@ -408,7 +406,7 @@ void RoundingImpl::apply(impl::DecimalQuantity &value, int32_t minInt, UErrorCod // This method is intended for the one specific purpose of helping print "00.000E0". U_ASSERT(isSignificantDigits()); U_ASSERT(value.isZero()); - value.setFractionLength(fPrecision.fUnion.fracSig.fMinSig - minInt, INT32_MAX); + value.setMinFraction(fPrecision.fUnion.fracSig.fMinSig - minInt); } #endif /* #if !UCONFIG_NO_FORMATTING */ diff --git a/icu4c/source/test/intltest/numbertest_decimalquantity.cpp b/icu4c/source/test/intltest/numbertest_decimalquantity.cpp index ac5e1a4db1f..9b395082d7f 100644 --- a/icu4c/source/test/intltest/numbertest_decimalquantity.cpp +++ b/icu4c/source/test/intltest/numbertest_decimalquantity.cpp @@ -77,27 +77,30 @@ void DecimalQuantityTest::checkDoubleBehavior(double d, bool explicitRequired) { void DecimalQuantityTest::testDecimalQuantityBehaviorStandalone() { UErrorCode status = U_ZERO_ERROR; DecimalQuantity fq; - assertToStringAndHealth(fq, u""); + assertToStringAndHealth(fq, u""); fq.setToInt(51423); - assertToStringAndHealth(fq, u""); + assertToStringAndHealth(fq, u""); fq.adjustMagnitude(-3); - assertToStringAndHealth(fq, u""); - fq.setToLong(999999999999000L); - assertToStringAndHealth(fq, u""); - fq.setIntegerLength(2, 5); - assertToStringAndHealth(fq, u""); - fq.setFractionLength(3, 6); - assertToStringAndHealth(fq, u""); + assertToStringAndHealth(fq, u""); + + fq.setToLong(90909090909000L); + assertToStringAndHealth(fq, u""); + fq.setMinInteger(2); + fq.applyMaxInteger(5); + assertToStringAndHealth(fq, u""); + fq.setMinFraction(3); + assertToStringAndHealth(fq, u""); + fq.setToDouble(987.654321); - assertToStringAndHealth(fq, u""); + assertToStringAndHealth(fq, u""); fq.roundToInfinity(); - assertToStringAndHealth(fq, u""); + assertToStringAndHealth(fq, u""); fq.roundToIncrement(0.005, RoundingMode::UNUM_ROUND_HALFEVEN, 3, status); assertSuccess("Rounding to increment", status); - assertToStringAndHealth(fq, u""); + assertToStringAndHealth(fq, u""); fq.roundToMagnitude(-2, RoundingMode::UNUM_ROUND_HALFEVEN, status); assertSuccess("Rounding to magnitude", status); - assertToStringAndHealth(fq, u""); + assertToStringAndHealth(fq, u""); } void DecimalQuantityTest::testSwitchStorage() { @@ -119,6 +122,15 @@ void DecimalQuantityTest::testSwitchStorage() { assertFalse("Should not be using byte array", fq.isUsingBytes()); assertEquals("Failed on round", u"1.23412341234E+16", fq.toScientificString()); assertHealth(fq); + // Bytes with popFromLeft + fq.setToDecNumber({"999999999999999999"}, status); + assertToStringAndHealth(fq, u""); + fq.applyMaxInteger(17); + assertToStringAndHealth(fq, u""); + fq.applyMaxInteger(16); + assertToStringAndHealth(fq, u""); + fq.applyMaxInteger(15); + assertToStringAndHealth(fq, u""); } void DecimalQuantityTest::testCopyMove() { @@ -127,21 +139,21 @@ void DecimalQuantityTest::testCopyMove() { DecimalQuantity a; a.setToLong(1234123412341234L); DecimalQuantity b = a; // copy constructor - assertToStringAndHealth(a, u""); - assertToStringAndHealth(b, u""); + assertToStringAndHealth(a, u""); + assertToStringAndHealth(b, u""); DecimalQuantity c(std::move(a)); // move constructor - assertToStringAndHealth(c, u""); + assertToStringAndHealth(c, u""); c.setToLong(54321L); - assertToStringAndHealth(c, u""); + assertToStringAndHealth(c, u""); c = b; // copy assignment - assertToStringAndHealth(b, u""); - assertToStringAndHealth(c, u""); + assertToStringAndHealth(b, u""); + assertToStringAndHealth(c, u""); b.setToLong(45678); c.setToLong(56789); c = std::move(b); // move assignment - assertToStringAndHealth(c, u""); + assertToStringAndHealth(c, u""); a = std::move(c); // move assignment to a defunct object - assertToStringAndHealth(a, u""); + assertToStringAndHealth(a, u""); } // Large numbers (requires byte allocation) @@ -150,21 +162,21 @@ void DecimalQuantityTest::testCopyMove() { DecimalQuantity a; a.setToDecNumber({"1234567890123456789", -1}, status); DecimalQuantity b = a; // copy constructor - assertToStringAndHealth(a, u""); - assertToStringAndHealth(b, u""); + assertToStringAndHealth(a, u""); + assertToStringAndHealth(b, u""); DecimalQuantity c(std::move(a)); // move constructor - assertToStringAndHealth(c, u""); + assertToStringAndHealth(c, u""); c.setToDecNumber({"9876543210987654321", -1}, status); - assertToStringAndHealth(c, u""); + assertToStringAndHealth(c, u""); c = b; // copy assignment - assertToStringAndHealth(b, u""); - assertToStringAndHealth(c, u""); + assertToStringAndHealth(b, u""); + assertToStringAndHealth(c, u""); b.setToDecNumber({"876543210987654321", -1}, status); c.setToDecNumber({"987654321098765432", -1}, status); c = std::move(b); // move assignment - assertToStringAndHealth(c, u""); + assertToStringAndHealth(c, u""); a = std::move(c); // move assignment to a defunct object - assertToStringAndHealth(a, u""); + assertToStringAndHealth(a, u""); } } @@ -364,8 +376,10 @@ void DecimalQuantityTest::testMaxDigits() { DecimalQuantity dq; dq.setToDouble(876.543); dq.roundToInfinity(); - dq.setIntegerLength(0, 2); - dq.setFractionLength(0, 2); + dq.setMinInteger(0); + dq.applyMaxInteger(2); + dq.setMinFraction(0); + dq.roundToMagnitude(-2, UNUM_ROUND_FLOOR, status); assertEquals("Should trim, toPlainString", "76.54", dq.toPlainString()); assertEquals("Should trim, toScientificString", "7.654E+1", dq.toScientificString()); assertEquals("Should trim, toLong", 76LL, dq.toLong(true)); @@ -376,9 +390,7 @@ void DecimalQuantityTest::testMaxDigits() { dq.toDecNum(dn, status); DecimalQuantity copy; copy.setToDecNum(dn, status); - if (!logKnownIssue("13701")) { - assertEquals("Should trim, toDecNum", "76.54", copy.toPlainString()); - } + assertEquals("Should trim, toDecNum", "76.54", copy.toPlainString()); } void DecimalQuantityTest::testNickelRounding() { diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity.java index f9d2da5e9cb..258274fdb6f 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity.java @@ -27,26 +27,31 @@ import com.ibm.icu.text.UFieldPosition; */ public interface DecimalQuantity extends PluralRules.IFixedDecimal { /** - * Sets the minimum and maximum integer digits that this {@link DecimalQuantity} should generate. + * Sets the minimum integer digits that this {@link DecimalQuantity} should generate. * This method does not perform rounding. * * @param minInt * The minimum number of integer digits. - * @param maxInt - * The maximum number of integer digits. */ - public void setIntegerLength(int minInt, int maxInt); + public void setMinInteger(int minInt); /** - * Sets the minimum and maximum fraction digits that this {@link DecimalQuantity} should generate. + * Sets the minimum fraction digits that this {@link DecimalQuantity} should generate. * This method does not perform rounding. * * @param minFrac * The minimum number of fraction digits. - * @param maxFrac - * The maximum number of fraction digits. */ - public void setFractionLength(int minFrac, int maxFrac); + public void setMinFraction(int minFrac); + + /** + * Truncates digits from the upper magnitude of the number in order to satisfy the + * specified maximum number of integer digits. + * + * @param maxInt + * The maximum number of integer digits. + */ + public void applyMaxInteger(int maxInt); /** * Rounds the number to a specified interval, such as 0.05. diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java index 138c2e9acf7..e8f9f2eda41 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java @@ -79,45 +79,18 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity { */ protected boolean isApproximate; - // Four positions: left optional '(', left required '[', right required ']', right optional ')'. - // These four positions determine which digits are displayed in the output string. They do NOT - // affect rounding. These positions are internal-only and can be specified only by the public - // endpoints like setFractionLength, setIntegerLength, and setSignificantDigits, among others. - // - // * Digits between lReqPos and rReqPos are in the "required zone" and are always displayed. - // * Digits between lOptPos and rOptPos but outside the required zone are in the "optional zone" - // and are displayed unless they are trailing off the left or right edge of the number and - // have a numerical value of zero. In order to be "trailing", the digits need to be beyond - // the decimal point in their respective directions. - // * Digits outside of the "optional zone" are never displayed. - // - // See the table below for illustrative examples. - // - // +---------+---------+---------+---------+------------+------------------------+--------------+ - // | lOptPos | lReqPos | rReqPos | rOptPos | number | positions | en-US string | - // +---------+---------+---------+---------+------------+------------------------+--------------+ - // | 5 | 2 | -1 | -5 | 1234.567 | ( 12[34.5]67 ) | 1,234.567 | - // | 3 | 2 | -1 | -5 | 1234.567 | 1(2[34.5]67 ) | 234.567 | - // | 3 | 2 | -1 | -2 | 1234.567 | 1(2[34.5]6)7 | 234.56 | - // | 6 | 4 | 2 | -5 | 123456789. | 123(45[67]89. ) | 456,789. | - // | 6 | 4 | 2 | 1 | 123456789. | 123(45[67]8)9. | 456,780. | - // | -1 | -1 | -3 | -4 | 0.123456 | 0.1([23]4)56 | .0234 | - // | 6 | 4 | -2 | -2 | 12.3 | ( [ 12.3 ]) | 0012.30 | - // +---------+---------+---------+---------+------------+------------------------+--------------+ - // - protected int lOptPos = Integer.MAX_VALUE; + // Positions to keep track of leading and trailing zeros. + // lReqPos is the magnitude of the first required leading zero. + // rReqPos is the magnitude of the last required trailing zero. protected int lReqPos = 0; protected int rReqPos = 0; - protected int rOptPos = Integer.MIN_VALUE; @Override public void copyFrom(DecimalQuantity _other) { copyBcdFrom(_other); DecimalQuantity_AbstractBCD other = (DecimalQuantity_AbstractBCD) _other; - lOptPos = other.lOptPos; lReqPos = other.lReqPos; rReqPos = other.rReqPos; - rOptPos = other.rOptPos; scale = other.scale; precision = other.precision; flags = other.flags; @@ -127,20 +100,17 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity { } public DecimalQuantity_AbstractBCD clear() { - lOptPos = Integer.MAX_VALUE; lReqPos = 0; rReqPos = 0; - rOptPos = Integer.MIN_VALUE; flags = 0; setBcdToZero(); // sets scale, precision, hasDouble, origDouble, origDelta, and BCD data return this; } @Override - public void setIntegerLength(int minInt, int maxInt) { + public void setMinInteger(int minInt) { // Validation should happen outside of DecimalQuantity, e.g., in the Rounder class. assert minInt >= 0; - assert maxInt >= minInt; // Special behavior: do not set minInt to be less than what is already set. // This is so significant digits rounding can set the integer length. @@ -149,30 +119,40 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity { } // Save values into internal state - // Negation is safe for minFrac/maxFrac because -Integer.MAX_VALUE > Integer.MIN_VALUE - lOptPos = maxInt; lReqPos = minInt; } @Override - public void setFractionLength(int minFrac, int maxFrac) { + public void setMinFraction(int minFrac) { // Validation should happen outside of DecimalQuantity, e.g., in the Rounder class. assert minFrac >= 0; - assert maxFrac >= minFrac; // Save values into internal state // Negation is safe for minFrac/maxFrac because -Integer.MAX_VALUE > Integer.MIN_VALUE rReqPos = -minFrac; - rOptPos = -maxFrac; + } + + @Override + public void applyMaxInteger(int maxInt) { + // Validation should happen outside of DecimalQuantity, e.g., in the Precision class. + assert maxInt >= 0; + + if (precision == 0) { + return; + } + + int magnitude = getMagnitude(); + if (maxInt <= magnitude) { + popFromLeft(magnitude - maxInt + 1); + compact(); + } } @Override public long getPositionFingerprint() { long fingerprint = 0; - fingerprint ^= lOptPos; fingerprint ^= (lReqPos << 16); fingerprint ^= ((long) rReqPos << 32); - fingerprint ^= ((long) rOptPos << 48); return fingerprint; } @@ -276,7 +256,7 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity { assert !isApproximate; int magnitude = scale + precision; - int result = (lReqPos > magnitude) ? lReqPos : (lOptPos < magnitude) ? lOptPos : magnitude; + int result = (lReqPos > magnitude) ? lReqPos : magnitude; return result - 1; } @@ -287,7 +267,7 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity { assert !isApproximate; int magnitude = scale; - int result = (rReqPos < magnitude) ? rReqPos : (rOptPos > magnitude) ? rOptPos : magnitude; + int result = (rReqPos < magnitude) ? rReqPos : magnitude; return result; } @@ -586,7 +566,7 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity { // Fallback behavior upon truncateIfOverflow is to truncate at 17 digits. assert(truncateIfOverflow || fitsInLong()); long result = 0L; - int upperMagnitude = Math.min(scale + precision, lOptPos) - 1; + int upperMagnitude = scale + precision - 1; if (truncateIfOverflow) { upperMagnitude = Math.min(upperMagnitude, 17); } @@ -607,7 +587,7 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity { public long toFractionLong(boolean includeTrailingZeros) { long result = 0L; int magnitude = -1; - int lowerMagnitude = Math.max(scale, rOptPos); + int lowerMagnitude = scale; if (includeTrailingZeros) { lowerMagnitude = Math.min(lowerMagnitude, rReqPos); } @@ -1055,8 +1035,8 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity { } // NOTE: It is not safe to add to lOptPos (aka maxInt) or subtract from // rOptPos (aka -maxFrac) due to overflow. - int upperPos = Math.min(precision + scale, lOptPos) - scale - 1; - int lowerPos = Math.max(scale, rOptPos) - scale; + int upperPos = precision - 1; + int lowerPos = 0; int p = upperPos; result.append((char) ('0' + getDigitPos(p))); if ((--p) >= lowerPos) { @@ -1105,10 +1085,8 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity { scale == _other.scale && precision == _other.precision && flags == _other.flags - && lOptPos == _other.lOptPos && lReqPos == _other.lReqPos && rReqPos == _other.rReqPos - && rOptPos == _other.rOptPos && isApproximate == _other.isApproximate; if (!basicEquals) { return false; @@ -1169,6 +1147,14 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity { */ protected abstract void shiftRight(int numDigits); + /** + * Directly removes digits from the front of the BCD list. + * Updates precision. + * + * CAUTION: it is the caller's responsibility to call {@link #compact} after this method. + */ + protected abstract void popFromLeft(int numDigits); + /** * Sets the internal representation to zero. Clears any values stored in scale, precision, hasDouble, * origDouble, origDelta, and BCD data. diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java index faa46281a1d..791538c9386 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java @@ -154,6 +154,19 @@ public final class DecimalQuantity_DualStorageBCD extends DecimalQuantity_Abstra precision -= numDigits; } + @Override + protected void popFromLeft(int numDigits) { + if (usingBytes) { + int i = precision - 1; + for (; i >= precision - numDigits; i--) { + bcdBytes[i] = 0; + } + } else { + bcdLong &= (1L << ((precision - numDigits) * 4)) - 1; + } + precision -= numDigits; + } + @Override protected void setBcdToZero() { if (usingBytes) { @@ -425,11 +438,9 @@ public final class DecimalQuantity_DualStorageBCD extends DecimalQuantity_Abstra @Override public String toString() { - return String.format("", - (lOptPos > 1000 ? "999" : String.valueOf(lOptPos)), + return String.format("", lReqPos, rReqPos, - (rOptPos < -1000 ? "-999" : String.valueOf(rOptPos)), (usingBytes ? "bytes" : "long"), (isNegative() ? "-" : ""), toNumberString()); diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java index 11f590f56c4..17fbe912d8b 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java @@ -99,9 +99,10 @@ class NumberFormatterImpl { MicroProps micros = microPropsGenerator.processQuantity(inValue); micros.rounder.apply(inValue); if (micros.integerWidth.maxInt == -1) { - inValue.setIntegerLength(micros.integerWidth.minInt, Integer.MAX_VALUE); + inValue.setMinInteger(micros.integerWidth.minInt); } else { - inValue.setIntegerLength(micros.integerWidth.minInt, micros.integerWidth.maxInt); + inValue.setMinInteger(micros.integerWidth.minInt); + inValue.applyMaxInteger(micros.integerWidth.maxInt); } return micros; } @@ -111,9 +112,10 @@ class NumberFormatterImpl { MicroProps micros = microPropsGenerator.processQuantity(inValue); micros.rounder.apply(inValue); if (micros.integerWidth.maxInt == -1) { - inValue.setIntegerLength(micros.integerWidth.minInt, Integer.MAX_VALUE); + inValue.setMinInteger(micros.integerWidth.minInt); } else { - inValue.setIntegerLength(micros.integerWidth.minInt, micros.integerWidth.maxInt); + inValue.setMinInteger(micros.integerWidth.minInt); + inValue.applyMaxInteger(micros.integerWidth.maxInt); } return micros; } diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/Precision.java b/icu4j/main/classes/core/src/com/ibm/icu/number/Precision.java index 375b535b90e..5e526ebe5fc 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/number/Precision.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/number/Precision.java @@ -609,7 +609,7 @@ public abstract class Precision implements Cloneable { @Override public void apply(DecimalQuantity value) { value.roundToInfinity(); - value.setFractionLength(0, Integer.MAX_VALUE); + value.setMinFraction(0); } } @@ -625,8 +625,7 @@ public abstract class Precision implements Cloneable { @Override public void apply(DecimalQuantity value) { value.roundToMagnitude(getRoundingMagnitudeFraction(maxFrac), mathContext); - value.setFractionLength(Math.max(0, -getDisplayMagnitudeFraction(minFrac)), - Integer.MAX_VALUE); + value.setMinFraction(Math.max(0, -getDisplayMagnitudeFraction(minFrac))); } } @@ -642,11 +641,10 @@ public abstract class Precision implements Cloneable { @Override public void apply(DecimalQuantity value) { value.roundToMagnitude(getRoundingMagnitudeSignificant(value, maxSig), mathContext); - value.setFractionLength(Math.max(0, -getDisplayMagnitudeSignificant(value, minSig)), - Integer.MAX_VALUE); + value.setMinFraction(Math.max(0, -getDisplayMagnitudeSignificant(value, minSig))); // Make sure that digits are displayed on zero. if (value.isZero() && minSig > 0) { - value.setIntegerLength(1, Integer.MAX_VALUE); + value.setMinInteger(1); } } @@ -656,7 +654,7 @@ public abstract class Precision implements Cloneable { */ public void apply(DecimalQuantity quantity, int minInt) { assert quantity.isZero(); - quantity.setFractionLength(minSig - minInt, Integer.MAX_VALUE); + quantity.setMinFraction(minSig - minInt); } } @@ -687,7 +685,7 @@ public abstract class Precision implements Cloneable { roundingMag = Math.min(roundingMag, candidate); } value.roundToMagnitude(roundingMag, mathContext); - value.setFractionLength(Math.max(0, -displayMag), Integer.MAX_VALUE); + value.setMinFraction(Math.max(0, -displayMag)); } } @@ -701,7 +699,7 @@ public abstract class Precision implements Cloneable { @Override public void apply(DecimalQuantity value) { value.roundToIncrement(increment, mathContext); - value.setFractionLength(increment.scale(), increment.scale()); + value.setMinFraction(increment.scale()); } } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/impl/number/DecimalQuantity_64BitBCD.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/impl/number/DecimalQuantity_64BitBCD.java index 54f5cec9ad5..ea84162a983 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/impl/number/DecimalQuantity_64BitBCD.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/impl/number/DecimalQuantity_64BitBCD.java @@ -81,6 +81,12 @@ public final class DecimalQuantity_64BitBCD extends DecimalQuantity_AbstractBCD precision -= numDigits; } + @Override + protected void popFromLeft(int numDigits) { + bcd &= (1L << ((precision - numDigits) * 4)) - 1; + precision -= numDigits; + } + @Override protected void setBcdToZero() { bcd = 0L; @@ -173,11 +179,9 @@ public final class DecimalQuantity_64BitBCD extends DecimalQuantity_AbstractBCD @Override public String toString() { return String.format( - "", - (lOptPos > 1000 ? "max" : String.valueOf(lOptPos)), + "", lReqPos, rReqPos, - (rOptPos < -1000 ? "min" : String.valueOf(rOptPos)), bcd, scale); } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/impl/number/DecimalQuantity_ByteArrayBCD.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/impl/number/DecimalQuantity_ByteArrayBCD.java index 7012e845253..de87080b796 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/impl/number/DecimalQuantity_ByteArrayBCD.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/impl/number/DecimalQuantity_ByteArrayBCD.java @@ -93,6 +93,15 @@ public final class DecimalQuantity_ByteArrayBCD extends DecimalQuantity_Abstract precision -= numDigits; } + @Override + protected void popFromLeft(int numDigits) { + int i = precision - 1; + for (; i >= precision - numDigits; i--) { + bcd[i] = 0; + } + precision -= numDigits; + } + @Override protected void setBcdToZero() { for (int i = 0; i < precision; i++) { @@ -221,11 +230,9 @@ public final class DecimalQuantity_ByteArrayBCD extends DecimalQuantity_Abstract sb.append(bcd[i]); } return String.format( - "", - (lOptPos > 1000 ? "max" : String.valueOf(lOptPos)), + "", lReqPos, rReqPos, - (rOptPos < -1000 ? "min" : String.valueOf(rOptPos)), sb, "E", scale); diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/impl/number/DecimalQuantity_SimpleStorage.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/impl/number/DecimalQuantity_SimpleStorage.java index 8a86b802fc4..994ca998e2c 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/impl/number/DecimalQuantity_SimpleStorage.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/impl/number/DecimalQuantity_SimpleStorage.java @@ -319,37 +319,39 @@ public class DecimalQuantity_SimpleStorage implements DecimalQuantity { } @Override - public void setIntegerLength(int minInt, int maxInt) { + public void setMinInteger(int minInt) { // Graceful failures for bogus input minInt = Math.max(0, minInt); - maxInt = Math.max(0, maxInt); - - // The minima must be less than or equal to the maxima - if (maxInt < minInt) { - minInt = maxInt; - } // Save values into internal state // Negation is safe for minFrac/maxFrac because -Integer.MAX_VALUE > Integer.MIN_VALUE - lOptPos = maxInt; lReqPos = minInt; } @Override - public void setFractionLength(int minFrac, int maxFrac) { + public void setMinFraction(int minFrac) { // Graceful failures for bogus input minFrac = Math.max(0, minFrac); - maxFrac = Math.max(0, maxFrac); - - // The minima must be less than or equal to the maxima - if (maxFrac < minFrac) { - minFrac = maxFrac; - } // Save values into internal state // Negation is safe for minFrac/maxFrac because -Integer.MAX_VALUE > Integer.MIN_VALUE rReqPos = -minFrac; - rOptPos = -maxFrac; + } + + @Override + public void applyMaxInteger(int maxInt) { + BigDecimal d; + if (primary != -1) { + d = BigDecimal.valueOf(primary).scaleByPowerOfTen(primaryScale); + } else { + d = fallback; + } + d = d.scaleByPowerOfTen(-maxInt).remainder(BigDecimal.ONE).scaleByPowerOfTen(maxInt); + if (primary != -1) { + primary = d.scaleByPowerOfTen(-primaryScale).longValueExact(); + } else { + fallback = d; + } } @Override diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/DecimalQuantityTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/DecimalQuantityTest.java index e8ec189af9b..faaa9b3cf48 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/DecimalQuantityTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/DecimalQuantityTest.java @@ -168,8 +168,8 @@ public class DecimalQuantityTest extends TestFmwk { DecimalQuantity q0 = rq.createCopy(); // Force an accurate double q0.roundToInfinity(); - q0.setIntegerLength(1, Integer.MAX_VALUE); - q0.setFractionLength(1, Integer.MAX_VALUE); + q0.setMinInteger(1); + q0.setMinFraction(1); String actual = q0.toPlainString(); assertEquals("Unexpected output from simple string conversion (" + q0 + ")", expected, actual); } @@ -305,6 +305,15 @@ public class DecimalQuantityTest extends TestFmwk { assertFalse("Should not be using byte array", fq.isUsingBytes()); assertEquals("Failed on round", "1.23412341234E+16", fq.toScientificString()); assertNull("Failed health check", fq.checkHealth()); + // Bytes with popFromLeft + fq.setToBigDecimal(new BigDecimal("999999999999999999")); + assertToStringAndHealth(fq, ""); + fq.applyMaxInteger(17); + assertToStringAndHealth(fq, ""); + fq.applyMaxInteger(16); + assertToStringAndHealth(fq, ""); + fq.applyMaxInteger(15); + assertToStringAndHealth(fq, ""); } @Test @@ -389,25 +398,26 @@ public class DecimalQuantityTest extends TestFmwk { @Test public void testDecimalQuantityBehaviorStandalone() { DecimalQuantity_DualStorageBCD fq = new DecimalQuantity_DualStorageBCD(); - assertToStringAndHealth(fq, ""); + assertToStringAndHealth(fq, ""); fq.setToInt(51423); - assertToStringAndHealth(fq, ""); + assertToStringAndHealth(fq, ""); fq.adjustMagnitude(-3); - assertToStringAndHealth(fq, ""); - fq.setToLong(999999999999000L); - assertToStringAndHealth(fq, ""); - fq.setIntegerLength(2, 5); - assertToStringAndHealth(fq, ""); - fq.setFractionLength(3, 6); - assertToStringAndHealth(fq, ""); + assertToStringAndHealth(fq, ""); + fq.setToLong(90909090909000L); + assertToStringAndHealth(fq, ""); + fq.setMinInteger(2); + fq.applyMaxInteger(5); + assertToStringAndHealth(fq, ""); + fq.setMinFraction(3); + assertToStringAndHealth(fq, ""); fq.setToDouble(987.654321); - assertToStringAndHealth(fq, ""); + assertToStringAndHealth(fq, ""); fq.roundToInfinity(); - assertToStringAndHealth(fq, ""); + assertToStringAndHealth(fq, ""); fq.roundToIncrement(new BigDecimal("0.005"), MATH_CONTEXT_HALF_EVEN); - assertToStringAndHealth(fq, ""); + assertToStringAndHealth(fq, ""); fq.roundToMagnitude(-2, MATH_CONTEXT_HALF_EVEN); - assertToStringAndHealth(fq, ""); + assertToStringAndHealth(fq, ""); } @Test @@ -501,8 +511,10 @@ public class DecimalQuantityTest extends TestFmwk { public void testMaxDigits() { DecimalQuantity_DualStorageBCD dq = new DecimalQuantity_DualStorageBCD(876.543); dq.roundToInfinity(); - dq.setIntegerLength(0, 2); - dq.setFractionLength(0, 2); + dq.setMinInteger(0); + dq.applyMaxInteger(2); + dq.setMinFraction(0); + dq.roundToMagnitude(-2, RoundingUtils.mathContextUnlimited(RoundingMode.FLOOR)); assertEquals("Should trim, toPlainString", "76.54", dq.toPlainString()); assertEquals("Should trim, toScientificString", "7.654E+1", dq.toScientificString()); assertEquals("Should trim, toLong", 76, dq.toLong(true)); -- 2.40.0