ICU-13701 Syncs C and J increment rounding. Checks for nickel sooner.

This commit is contained in:
Shane Carr 2018-11-02 22:19:53 -07:00 committed by Shane F. Carr
parent a1cc16ccd3
commit ba21ff79c4
14 changed files with 226 additions and 49 deletions

View File

@ -175,33 +175,28 @@ uint64_t DecimalQuantity::getPositionFingerprint() const {
}
void DecimalQuantity::roundToIncrement(double roundingIncrement, RoundingMode roundingMode,
int32_t maxFrac, UErrorCode& status) {
// TODO(13701): Move the nickel check into a higher-level API.
if (roundingIncrement == 0.05) {
roundToMagnitude(-2, roundingMode, true, status);
roundToMagnitude(-maxFrac, roundingMode, false, status);
return;
} else if (roundingIncrement == 0.5) {
roundToMagnitude(-1, roundingMode, true, status);
roundToMagnitude(-maxFrac, roundingMode, false, status);
return;
}
// TODO(13701): This is innefficient. Improve?
// TODO(13701): Should we convert to decNumber instead?
roundToInfinity();
double temp = toDouble();
temp /= roundingIncrement;
// Use another DecimalQuantity to perform the actual rounding...
DecimalQuantity dq;
dq.setToDouble(temp);
dq.roundToMagnitude(0, roundingMode, status);
temp = dq.toDouble();
temp *= roundingIncrement;
setToDouble(temp);
// Since we reset the value to a double, we need to specify the rounding boundary
// in order to get the DecimalQuantity out of approximation mode.
// NOTE: In Java, we have minMaxFrac, but in C++, the two are differentiated.
roundToMagnitude(-maxFrac, roundingMode, status);
UErrorCode& status) {
// Do not call this method with an increment having only a 1 or a 5 digit!
// Use a more efficient call to either roundToMagnitude() or roundToNickel().
// Check a few popular rounding increments; a more thorough check is in Java.
U_ASSERT(roundingIncrement != 0.01);
U_ASSERT(roundingIncrement != 0.05);
U_ASSERT(roundingIncrement != 0.1);
U_ASSERT(roundingIncrement != 0.5);
U_ASSERT(roundingIncrement != 1);
U_ASSERT(roundingIncrement != 5);
DecNum incrementDN;
incrementDN.setTo(roundingIncrement, status);
if (U_FAILURE(status)) { return; }
// Divide this DecimalQuantity by the increment, round, then multiply back.
divideBy(incrementDN, status);
if (U_FAILURE(status)) { return; }
roundToMagnitude(0, roundingMode, status);
if (U_FAILURE(status)) { return; }
multiplyBy(incrementDN, status);
if (U_FAILURE(status)) { return; }
}
void DecimalQuantity::multiplyBy(const DecNum& multiplicand, UErrorCode& status) {

View File

@ -85,7 +85,7 @@ class U_I18N_API DecimalQuantity : public IFixedDecimal, public UMemory {
* @param roundingMode The {@link RoundingMode} to use if rounding is necessary.
*/
void roundToIncrement(double roundingIncrement, RoundingMode roundingMode,
int32_t maxFrac, UErrorCode& status);
UErrorCode& status);
/** Removes all fraction digits. */
void truncate();

View File

@ -295,7 +295,9 @@ MacroProps NumberPropertyMapper::oldToNew(const DecimalFormatProperties& propert
if (rounding_.fType == Precision::PrecisionType::RND_FRACTION) {
minFrac_ = rounding_.fUnion.fracSig.fMinFrac;
maxFrac_ = rounding_.fUnion.fracSig.fMaxFrac;
} else if (rounding_.fType == Precision::PrecisionType::RND_INCREMENT) {
} else if (rounding_.fType == Precision::PrecisionType::RND_INCREMENT
|| rounding_.fType == Precision::PrecisionType::RND_INCREMENT_ONE
|| rounding_.fType == Precision::PrecisionType::RND_INCREMENT_FIVE) {
increment_ = rounding_.fUnion.increment.fIncrement;
minFrac_ = rounding_.fUnion.increment.fMinFrac;
maxFrac_ = rounding_.fUnion.increment.fMinFrac;

View File

@ -716,7 +716,7 @@ UnicodeString PatternStringUtils::propertiesToPatternString(const DecimalFormatP
}
} else if (roundingInterval != 0.0) {
// Rounding Interval.
digitsStringScale = -roundingutils::doubleFractionLength(roundingInterval);
digitsStringScale = -roundingutils::doubleFractionLength(roundingInterval, nullptr);
// TODO: Check for DoS here?
DecimalQuantity incrementQuantity;
incrementQuantity.setToDouble(roundingInterval);

View File

@ -55,7 +55,7 @@ int32_t getDisplayMagnitudeSignificant(const DecimalQuantity &value, int minSig)
MultiplierProducer::~MultiplierProducer() = default;
digits_t roundingutils::doubleFractionLength(double input) {
digits_t roundingutils::doubleFractionLength(double input, int8_t* singleDigit) {
char buffer[DoubleToStringConverter::kBase10MaximalLength + 1];
bool sign; // unused; always positive
int32_t length;
@ -71,6 +71,14 @@ digits_t roundingutils::doubleFractionLength(double input) {
&point
);
if (singleDigit == nullptr) {
// no-op
} else if (length == 1) {
*singleDigit = buffer[0] - '0';
} else {
*singleDigit = -1;
}
return static_cast<digits_t>(length - point);
}
@ -254,14 +262,27 @@ Precision::constructFractionSignificant(const FractionPrecision &base, int32_t m
IncrementPrecision Precision::constructIncrement(double increment, int32_t minFrac) {
IncrementSettings settings;
// Note: For number formatting, fIncrement is used for RND_INCREMENT but not
// RND_INCREMENT_ONE or RND_INCREMENT_FIVE. However, fIncrement is used in all
// three when constructing a skeleton.
settings.fIncrement = increment;
settings.fMinFrac = static_cast<digits_t>(minFrac);
// One of the few pre-computed quantities:
// Note: it is possible for minFrac to be more than maxFrac... (misleading)
settings.fMaxFrac = roundingutils::doubleFractionLength(increment);
int8_t singleDigit;
settings.fMaxFrac = roundingutils::doubleFractionLength(increment, &singleDigit);
PrecisionUnion union_;
union_.increment = settings;
return {RND_INCREMENT, union_, kDefaultMode};
if (singleDigit == 1) {
// NOTE: In C++, we must return the correct value type with the correct union.
// It would be invalid to return a RND_FRACTION here because the methods on the
// IncrementPrecision type assume that the union is backed by increment data.
return {RND_INCREMENT_ONE, union_, kDefaultMode};
} else if (singleDigit == 5) {
return {RND_INCREMENT_FIVE, union_, kDefaultMode};
} else {
return {RND_INCREMENT, union_, kDefaultMode};
}
}
CurrencyPrecision Precision::constructCurrency(UCurrencyUsage usage) {
@ -390,7 +411,22 @@ void RoundingImpl::apply(impl::DecimalQuantity &value, UErrorCode& status) const
value.roundToIncrement(
fPrecision.fUnion.increment.fIncrement,
fRoundingMode,
fPrecision.fUnion.increment.fMaxFrac,
status);
value.setMinFraction(fPrecision.fUnion.increment.fMinFrac);
break;
case Precision::RND_INCREMENT_ONE:
value.roundToMagnitude(
-fPrecision.fUnion.increment.fMaxFrac,
fRoundingMode,
status);
value.setMinFraction(fPrecision.fUnion.increment.fMinFrac);
break;
case Precision::RND_INCREMENT_FIVE:
value.roundToNickel(
-fPrecision.fUnion.increment.fMaxFrac,
fRoundingMode,
status);
value.setMinFraction(fPrecision.fUnion.increment.fMinFrac);
break;
@ -399,6 +435,10 @@ void RoundingImpl::apply(impl::DecimalQuantity &value, UErrorCode& status) const
// Call .withCurrency() before .apply()!
U_ASSERT(false);
break;
default:
U_ASSERT(false);
break;
}
}

View File

@ -134,8 +134,11 @@ inline bool roundsAtMidpoint(int roundingMode) {
/**
* Computes the number of fraction digits in a double. Used for computing maxFrac for an increment.
* Calls into the DoubleToStringConverter library to do so.
*
* @param singleDigit An output parameter; set to a number if that is the
* only digit in the double, or -1 if there is more than one digit.
*/
digits_t doubleFractionLength(double input);
digits_t doubleFractionLength(double input, int8_t* singleDigit);
} // namespace roundingutils

View File

@ -1389,7 +1389,9 @@ bool GeneratorHelpers::precision(const MacroProps& macros, UnicodeString& sb, UE
} else {
blueprint_helpers::generateDigitsStem(impl.fMinSig, -1, sb, status);
}
} else if (macros.precision.fType == Precision::RND_INCREMENT) {
} else if (macros.precision.fType == Precision::RND_INCREMENT
|| macros.precision.fType == Precision::RND_INCREMENT_ONE
|| macros.precision.fType == Precision::RND_INCREMENT_FIVE) {
const Precision::IncrementSettings& impl = macros.precision.fUnion.increment;
sb.append(u"precision-increment/", -1);
blueprint_helpers::generateIncrementOption(

View File

@ -237,7 +237,9 @@ void DecNum::multiplyBy(const DecNum& rhs, UErrorCode& status) {
void DecNum::divideBy(const DecNum& rhs, UErrorCode& status) {
uprv_decNumberDivide(fData, fData, rhs.fData, &fContext);
if (fContext.status != 0) {
if ((fContext.status & DEC_Inexact) != 0) {
// Ignore.
} else if (fContext.status != 0) {
status = U_INTERNAL_PROGRAM_ERROR;
}
}

View File

@ -715,7 +715,18 @@ class U_I18N_API Precision : public UMemory {
RND_FRACTION,
RND_SIGNIFICANT,
RND_FRACTION_SIGNIFICANT,
// Used for strange increments like 3.14.
RND_INCREMENT,
// Used for increments with 1 as the only digit. This is different than fraction
// rounding because it supports having additional trailing zeros. For example, this
// class is used to round with the increment 0.010.
RND_INCREMENT_ONE,
// Used for increments with 5 as the only digit (nickel rounding).
RND_INCREMENT_FIVE,
RND_CURRENCY,
RND_ERROR
} fType;
@ -735,13 +746,14 @@ class U_I18N_API Precision : public UMemory {
} fracSig;
/** @internal */
struct IncrementSettings {
// For RND_INCREMENT, RND_INCREMENT_ONE, and RND_INCREMENT_FIVE
/** @internal */
double fIncrement;
/** @internal */
impl::digits_t fMinFrac;
/** @internal */
impl::digits_t fMaxFrac;
} increment; // For RND_INCREMENT
} increment;
UCurrencyUsage currencyUsage; // For RND_CURRENCY
UErrorCode errorCode; // For RND_ERROR
} fUnion;

View File

@ -1078,6 +1078,36 @@ void NumberFormatterApiTest::roundingOther() {
u"0.00",
u"0.00");
assertFormatDescending(
u"Strange Increment",
u"precision-increment/3.140",
NumberFormatter::with().precision(Precision::increment(3.14).withMinFraction(3)),
Locale::getEnglish(),
u"87,649.960",
u"8,763.740",
u"876.060",
u"87.920",
u"9.420",
u"0.000",
u"0.000",
u"0.000",
u"0.000");
assertFormatDescending(
u"Increment Resolving to Power of 10",
u"precision-increment/0.010",
NumberFormatter::with().precision(Precision::increment(0.01).withMinFraction(3)),
Locale::getEnglish(),
u"87,650.000",
u"8,765.000",
u"876.500",
u"87.650",
u"8.760",
u"0.880",
u"0.090",
u"0.010",
u"0.000");
assertFormatDescending(
u"Currency Standard",
u"currency/CZK precision-currency-standard",

View File

@ -95,7 +95,7 @@ void DecimalQuantityTest::testDecimalQuantityBehaviorStandalone() {
assertToStringAndHealth(fq, u"<DecimalQuantity 2:-3 long 987654321E-6>");
fq.roundToInfinity();
assertToStringAndHealth(fq, u"<DecimalQuantity 2:-3 long 987654321E-6>");
fq.roundToIncrement(0.005, RoundingMode::UNUM_ROUND_HALFEVEN, 3, status);
fq.roundToIncrement(0.005, RoundingMode::UNUM_ROUND_HALFEVEN, status);
assertSuccess("Rounding to increment", status);
assertToStringAndHealth(fq, u"<DecimalQuantity 2:-3 long 987655E-3>");
fq.roundToMagnitude(-2, RoundingMode::UNUM_ROUND_HALFEVEN, status);

View File

@ -158,15 +158,17 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity {
@Override
public void roundToIncrement(BigDecimal roundingIncrement, MathContext mathContext) {
// TODO(13701): Avoid this check on every call to roundToIncrement().
BigDecimal stripped = roundingIncrement.stripTrailingZeros();
if (stripped.unscaledValue().compareTo(BigInteger.valueOf(5)) == 0) {
roundToNickel(-stripped.scale(), mathContext);
return;
}
// Do not call this method with an increment having only a 1 or a 5 digit!
// Use a more efficient call to either roundToMagnitude() or roundToNickel().
// Note: The check, which is somewhat expensive, is performed in an assertion
// to disable it in production.
assert roundingIncrement.stripTrailingZeros().precision() != 1
|| roundingIncrement.stripTrailingZeros().unscaledValue().intValue() != 5
|| roundingIncrement.stripTrailingZeros().unscaledValue().intValue() != 1;
BigDecimal temp = toBigDecimal();
temp = temp.divide(roundingIncrement, 0, mathContext.getRoundingMode())
.multiply(roundingIncrement).round(mathContext);
.multiply(roundingIncrement)
.round(mathContext);
if (temp.signum() == 0) {
setBcdToZero(); // keeps negative flag for -0.0
} else {

View File

@ -3,6 +3,7 @@
package com.ibm.icu.number;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.math.MathContext;
import java.math.RoundingMode;
@ -445,7 +446,7 @@ public abstract class Precision implements Cloneable {
static final FracSigRounderImpl COMPACT_STRATEGY = new FracSigRounderImpl(0, 0, 2, -1);
static final IncrementRounderImpl NICKEL = new IncrementRounderImpl(BigDecimal.valueOf(0.05));
static final IncrementFiveRounderImpl NICKEL = new IncrementFiveRounderImpl(new BigDecimal("0.05"), 2, 2);
static final CurrencyRounderImpl MONETARY_STANDARD = new CurrencyRounderImpl(CurrencyUsage.STANDARD);
static final CurrencyRounderImpl MONETARY_CASH = new CurrencyRounderImpl(CurrencyUsage.CASH);
@ -495,9 +496,22 @@ public abstract class Precision implements Cloneable {
// NOTE: .equals() is what we want, not .compareTo()
if (increment.equals(NICKEL.increment)) {
return NICKEL;
} else {
return new IncrementRounderImpl(increment);
}
// Note: For number formatting, the BigDecimal increment is used for IncrementRounderImpl
// but not mIncrementOneRounderImpl or IncrementFiveRounderImpl. However, fIncrement is
// used in all three when constructing a skeleton.
BigDecimal reduced = increment.stripTrailingZeros();
if (reduced.precision() == 1) {
int minFrac = increment.scale();
int maxFrac = reduced.scale();
BigInteger digit = reduced.unscaledValue();
if (digit.intValue() == 1) {
return new IncrementOneRounderImpl(increment, minFrac, maxFrac);
} else if (digit.intValue() == 5) {
return new IncrementFiveRounderImpl(increment, minFrac, maxFrac);
}
}
return new IncrementRounderImpl(increment);
}
static CurrencyPrecision constructCurrency(CurrencyUsage usage) {
@ -689,6 +703,9 @@ public abstract class Precision implements Cloneable {
}
}
/**
* Used for strange increments like 3.14.
*/
static class IncrementRounderImpl extends Precision {
final BigDecimal increment;
@ -703,6 +720,48 @@ public abstract class Precision implements Cloneable {
}
}
/**
* Used for increments with 1 as the only digit. This is different than fraction
* rounding because it supports having additional trailing zeros. For example, this
* class is used to round with the increment 0.010.
*/
static class IncrementOneRounderImpl extends IncrementRounderImpl {
final int minFrac;
final int maxFrac;
public IncrementOneRounderImpl(BigDecimal increment, int minFrac, int maxFrac) {
super(increment);
this.minFrac = minFrac;
this.maxFrac = maxFrac;
}
@Override
public void apply(DecimalQuantity value) {
value.roundToMagnitude(-maxFrac, mathContext);
value.setMinFraction(minFrac);
}
}
/**
* Used for increments with 5 as the only digit (nickel rounding).
*/
static class IncrementFiveRounderImpl extends IncrementRounderImpl {
final int minFrac;
final int maxFrac;
public IncrementFiveRounderImpl(BigDecimal increment, int minFrac, int maxFrac) {
super(increment);
this.minFrac = minFrac;
this.maxFrac = maxFrac;
}
@Override
public void apply(DecimalQuantity value) {
value.roundToNickel(-maxFrac, mathContext);
value.setMinFraction(minFrac);
}
}
static class CurrencyRounderImpl extends CurrencyPrecision {
final CurrencyUsage usage;

View File

@ -1078,6 +1078,36 @@ public class NumberFormatterApiTest {
"0.00",
"0.00");
assertFormatDescending(
"Strange Increment",
"precision-increment/3.140",
NumberFormatter.with().precision(Precision.increment(new BigDecimal("3.140"))),
ULocale.ENGLISH,
"87,649.960",
"8,763.740",
"876.060",
"87.920",
"9.420",
"0.000",
"0.000",
"0.000",
"0.000");
assertFormatDescending(
"Increment Resolving to Power of 10",
"precision-increment/0.010",
NumberFormatter.with().precision(Precision.increment(new BigDecimal("0.010"))),
ULocale.ENGLISH,
"87,650.000",
"8,765.000",
"876.500",
"87.650",
"8.760",
"0.880",
"0.090",
"0.010",
"0.000");
assertFormatDescending(
"Currency Standard",
"currency/CZK precision-currency-standard",