ICU-20568 Fix "1 foot 12 inches" behaviour
See #1337
diff --git a/icu4c/source/i18n/number_rounding.cpp b/icu4c/source/i18n/number_rounding.cpp
index 1489667..a8fd6bc 100644
--- a/icu4c/source/i18n/number_rounding.cpp
+++ b/icu4c/source/i18n/number_rounding.cpp
@@ -24,10 +24,9 @@
using double_conversion::DoubleToStringConverter;
using icu::StringSegment;
-// Most blueprint_helpers live in number_skeletons.cpp. This one is in
-// number_rounding.cpp for dependency reasons.
-void blueprint_helpers::parseIncrementOption(const StringSegment &segment, MacroProps ¯os,
- UErrorCode &status) {
+void number::impl::parseIncrementOption(const StringSegment &segment,
+ Precision &outPrecision,
+ UErrorCode &status) {
// Need to do char <-> UChar conversion...
U_ASSERT(U_SUCCESS(status));
CharString buffer;
@@ -50,10 +49,10 @@
decimalOffset++;
}
if (decimalOffset == segment.length()) {
- macros.precision = Precision::increment(increment);
+ outPrecision = Precision::increment(increment);
} else {
int32_t fractionLength = segment.length() - decimalOffset - 1;
- macros.precision = Precision::increment(increment).withMinFraction(fractionLength);
+ outPrecision = Precision::increment(increment).withMinFraction(fractionLength);
}
}
diff --git a/icu4c/source/i18n/number_roundingutils.h b/icu4c/source/i18n/number_roundingutils.h
index c6e5c77..e85cbae 100644
--- a/icu4c/source/i18n/number_roundingutils.h
+++ b/icu4c/source/i18n/number_roundingutils.h
@@ -8,6 +8,7 @@
#define __NUMBER_ROUNDINGUTILS_H__
#include "number_types.h"
+#include "string_segment.h"
U_NAMESPACE_BEGIN
namespace number {
@@ -192,12 +193,20 @@
bool fPassThrough = true; // default value
// Permits access to fPrecision.
- friend class UsagePrefsHandler;
+ friend class units::UnitsRouter;
// Permits access to fPrecision.
friend class UnitConversionHandler;
};
+/**
+ * Parses Precision-related skeleton strings without knowledge of MacroProps
+ * - see blueprint_helpers::parseIncrementOption().
+ *
+ * Referencing MacroProps means needing to pull in the .o files that have the
+ * destructors for the SymbolsWrapper, Usage, and Scale classes.
+ */
+void parseIncrementOption(const StringSegment &segment, Precision &outPrecision, UErrorCode &status);
} // namespace impl
} // namespace number
diff --git a/icu4c/source/i18n/number_skeletons.cpp b/icu4c/source/i18n/number_skeletons.cpp
index 5a23bd3..fd7d7ca 100644
--- a/icu4c/source/i18n/number_skeletons.cpp
+++ b/icu4c/source/i18n/number_skeletons.cpp
@@ -10,6 +10,7 @@
#define UNISTR_FROM_STRING_EXPLICIT
#include "number_decnum.h"
+#include "number_roundingutils.h"
#include "number_skeletons.h"
#include "umutex.h"
#include "ucln_in.h"
@@ -1333,8 +1334,10 @@
return true;
}
-// blueprint_helpers::parseIncrementOption lives in number_rounding.cpp for
-// dependencies reasons.
+void blueprint_helpers::parseIncrementOption(const StringSegment &segment, MacroProps ¯os,
+ UErrorCode &status) {
+ number::impl::parseIncrementOption(segment, macros.precision, status);
+}
void blueprint_helpers::generateIncrementOption(double increment, int32_t trailingZeros, UnicodeString& sb,
UErrorCode&) {
diff --git a/icu4c/source/i18n/number_usageprefs.cpp b/icu4c/source/i18n/number_usageprefs.cpp
index 494b968..0d9cb06 100644
--- a/icu4c/source/i18n/number_usageprefs.cpp
+++ b/icu4c/source/i18n/number_usageprefs.cpp
@@ -157,7 +157,7 @@
}
quantity.roundToInfinity(); // Enables toDouble
- const auto routed = fUnitsRouter.route(quantity.toDouble(), status);
+ const units::RouteResult routed = fUnitsRouter.route(quantity.toDouble(), µs.rounder, status);
if (U_FAILURE(status)) {
return;
}
@@ -168,38 +168,6 @@
}
mixedMeasuresToMicros(routedMeasures, &quantity, µs, status);
-
- UnicodeString precisionSkeleton = routed.precision;
- if (micros.rounder.fPrecision.isBogus()) {
- if (precisionSkeleton.length() > 0) {
- micros.rounder.fPrecision = parseSkeletonToPrecision(precisionSkeleton, status);
- } else {
- // We use the same rounding mode as COMPACT notation: known to be a
- // human-friendly rounding mode: integers, but add a decimal digit
- // as needed to ensure we have at least 2 significant digits.
- micros.rounder.fPrecision = Precision::integer().withMinDigits(2);
- }
- }
-}
-
-Precision UsagePrefsHandler::parseSkeletonToPrecision(icu::UnicodeString precisionSkeleton,
- UErrorCode status) {
- if (U_FAILURE(status)) {
- // As a member of UsagePrefsHandler, which is a friend of Precision, we
- // get access to the default constructor.
- return {};
- }
- constexpr int32_t kSkelPrefixLen = 20;
- if (!precisionSkeleton.startsWith(UNICODE_STRING_SIMPLE("precision-increment/"))) {
- status = U_INVALID_FORMAT_ERROR;
- return {};
- }
- U_ASSERT(precisionSkeleton[kSkelPrefixLen - 1] == u'/');
- StringSegment segment(precisionSkeleton, false);
- segment.adjustOffset(kSkelPrefixLen);
- MacroProps macros;
- blueprint_helpers::parseIncrementOption(segment, macros, status);
- return macros.precision;
}
UnitConversionHandler::UnitConversionHandler(const MeasureUnit &inputUnit, const MeasureUnit &outputUnit,
@@ -227,19 +195,14 @@
return;
}
quantity.roundToInfinity(); // Enables toDouble
- MaybeStackVector<Measure> measures = fUnitConverter->convert(quantity.toDouble(), status);
+ MaybeStackVector<Measure> measures =
+ fUnitConverter->convert(quantity.toDouble(), µs.rounder, status);
micros.outputUnit = fOutputUnit;
if (U_FAILURE(status)) {
return;
}
mixedMeasuresToMicros(measures, &quantity, µs, status);
-
- // TODO: add tests to explore behaviour that may suggest a more
- // human-centric default rounder?
- // if (micros.rounder.fPrecision.isBogus()) {
- // micros.rounder.fPrecision = Precision::integer().withMinDigits(2);
- // }
}
#endif /* #if !UCONFIG_NO_FORMATTING */
diff --git a/icu4c/source/i18n/number_usageprefs.h b/icu4c/source/i18n/number_usageprefs.h
index b773979..9e8bd93 100644
--- a/icu4c/source/i18n/number_usageprefs.h
+++ b/icu4c/source/i18n/number_usageprefs.h
@@ -60,8 +60,6 @@
private:
UnitsRouter fUnitsRouter;
const MicroPropsGenerator *fParent;
-
- static Precision parseSkeletonToPrecision(icu::UnicodeString precisionSkeleton, UErrorCode status);
};
} // namespace impl
diff --git a/icu4c/source/i18n/unicode/numberformatter.h b/icu4c/source/i18n/unicode/numberformatter.h
index c5aeeca..b0c63dc 100644
--- a/icu4c/source/i18n/unicode/numberformatter.h
+++ b/icu4c/source/i18n/unicode/numberformatter.h
@@ -99,6 +99,13 @@
}
}
+namespace units {
+
+// Forward declarations:
+class UnitsRouter;
+
+} // namespace units
+
namespace number { // icu::number
// Forward declarations:
@@ -158,7 +165,6 @@
class MutablePatternModifier;
class ImmutablePatternModifier;
struct DecimalFormatWarehouse;
-class UsagePrefsHandler;
/**
* Used for NumberRangeFormatter and implemented in numrange_fluent.cpp.
@@ -764,7 +770,7 @@
friend class impl::GeneratorHelpers;
// To allow access to isBogus and the default (bogus) constructor:
- friend class impl::UsagePrefsHandler;
+ friend class units::UnitsRouter;
};
/**
diff --git a/icu4c/source/i18n/units_complexconverter.cpp b/icu4c/source/i18n/units_complexconverter.cpp
index f145030..9775b6a 100644
--- a/icu4c/source/i18n/units_complexconverter.cpp
+++ b/icu4c/source/i18n/units_complexconverter.cpp
@@ -8,6 +8,8 @@
#include <cmath>
#include "cmemory.h"
+#include "number_decimalquantity.h"
+#include "number_roundingutils.h"
#include "uarrsort.h"
#include "uassert.h"
#include "unicode/fmtable.h"
@@ -105,11 +107,24 @@
return newQuantity >= limit;
}
-MaybeStackVector<Measure> ComplexUnitsConverter::convert(double quantity, UErrorCode &status) const {
+MaybeStackVector<Measure> ComplexUnitsConverter::convert(double quantity,
+ icu::number::impl::RoundingImpl *rounder,
+ UErrorCode &status) const {
// TODO(icu-units#63): test negative numbers!
// TODO(hugovdm): return an error for "foot-and-foot"?
MaybeStackVector<Measure> result;
+ // For N converters:
+ // - the first converter converts from the input unit to the largest unit,
+ // - N-1 converters convert to bigger units for which we want integers,
+ // - the Nth converter (index N-1) converts to the smallest unit, for which
+ // we keep a double.
+ MaybeStackArray<int64_t, 5> intValues(unitConverters_.length() - 1, status);
+ if (U_FAILURE(status)) {
+ return result;
+ }
+ uprv_memset(intValues.getAlias(), 0, (unitConverters_.length() - 1) * sizeof(int64_t));
+
for (int i = 0, n = unitConverters_.length(); i < n; ++i) {
quantity = (*unitConverters_[i]).convert(quantity);
if (i < n - 1) {
@@ -120,11 +135,7 @@
// original values to ensure unbiased accuracy (to the extent of
// double's capabilities).
int64_t roundedQuantity = floor(quantity * (1 + DBL_EPSILON));
- Formattable formattableNewQuantity(roundedQuantity);
-
- // NOTE: Measure would own its MeasureUnit.
- MeasureUnit *type = new MeasureUnit(units_[i]->copy(status).build(status));
- result.emplaceBackAndCheckErrorCode(status, formattableNewQuantity, type, status);
+ intValues[i] = roundedQuantity;
// Keep the residual of the quantity.
// For example: `3.6 feet`, keep only `0.6 feet`
@@ -137,11 +148,76 @@
quantity -= roundedQuantity;
}
} else { // LAST ELEMENT
- Formattable formattableQuantity(quantity);
+ if (rounder == nullptr) {
+ // Nothing to do for the last element.
+ break;
+ }
- // NOTE: Measure would own its MeasureUnit.
+ // Round the last value
+ // TODO(ICU-21288): get smarter about precision for mixed units.
+ number::impl::DecimalQuantity quant;
+ quant.setToDouble(quantity);
+ rounder->apply(quant, status);
+ if (U_FAILURE(status)) {
+ return result;
+ }
+ quantity = quant.toDouble();
+ if (i == 0) {
+ // Last element is also the first element, so we're done
+ break;
+ }
+
+ // Check if there's a carry, and bubble it back up the resulting intValues.
+ int64_t carry = floor(unitConverters_[i]->convertInverse(quantity) * (1 + DBL_EPSILON));
+ if (carry <= 0) {
+ break;
+ }
+ quantity -= unitConverters_[i]->convert(carry);
+ intValues[i - 1] += carry;
+
+ // We don't use the first converter: that one is for the input unit
+ for (int32_t j = i - 1; j > 0; j--) {
+ carry = floor(unitConverters_[j]->convertInverse(intValues[j]) * (1 + DBL_EPSILON));
+ if (carry <= 0) {
+ break;
+ }
+ intValues[j] -= round(unitConverters_[j]->convert(carry));
+ intValues[j - 1] += carry;
+ }
+ }
+ }
+
+ // Package values into Measure instances in result:
+ for (int i = 0, n = unitConverters_.length(); i < n; ++i) {
+ if (i < n - 1) {
+ Formattable formattableQuantity(intValues[i]);
+ // Measure takes ownership of the MeasureUnit*
MeasureUnit *type = new MeasureUnit(units_[i]->copy(status).build(status));
- result.emplaceBackAndCheckErrorCode(status, formattableQuantity, type, status);
+ if (result.emplaceBackAndCheckErrorCode(status, formattableQuantity, type, status) ==
+ nullptr) {
+ // Ownership wasn't taken
+ U_ASSERT(U_FAILURE(status));
+ delete type;
+ }
+ if (U_FAILURE(status)) {
+ return result;
+ }
+ } else { // LAST ELEMENT
+ // Add the last element, not an integer:
+ Formattable formattableQuantity(quantity);
+ // Measure takes ownership of the MeasureUnit*
+ MeasureUnit *type = new MeasureUnit(units_[i]->copy(status).build(status));
+ if (result.emplaceBackAndCheckErrorCode(status, formattableQuantity, type, status) ==
+ nullptr) {
+ // Ownership wasn't taken
+ U_ASSERT(U_FAILURE(status));
+ delete type;
+ }
+ if (U_FAILURE(status)) {
+ return result;
+ }
+ U_ASSERT(result.length() == i + 1);
+ U_ASSERT(result[i] != nullptr);
}
}
@@ -153,6 +229,7 @@
for (int32_t i = 0; i < unitsCount; i++) {
for (int32_t j = i; j < unitsCount; j++) {
// Find the next expected unit, and swap it into place.
+ U_ASSERT(result[j] != nullptr);
if (result[j]->getUnit() == *outputUnits_[i]) {
if (j != i) {
Measure *tmp = arr[j];
diff --git a/icu4c/source/i18n/units_complexconverter.h b/icu4c/source/i18n/units_complexconverter.h
index fc355d9..83c5b94 100644
--- a/icu4c/source/i18n/units_complexconverter.h
+++ b/icu4c/source/i18n/units_complexconverter.h
@@ -9,6 +9,7 @@
#include "cmemory.h"
#include "measunit_impl.h"
+#include "number_roundingutils.h"
#include "unicode/errorcode.h"
#include "unicode/measure.h"
#include "units_converter.h"
@@ -73,7 +74,8 @@
// NOTE:
// the smallest element is the only element that could have fractional values. And all
// other elements are floored to the nearest integer
- MaybeStackVector<Measure> convert(double quantity, UErrorCode &status) const;
+ MaybeStackVector<Measure>
+ convert(double quantity, icu::number::impl::RoundingImpl *rounder, UErrorCode &status) const;
private:
MaybeStackVector<UnitConverter> unitConverters_;
diff --git a/icu4c/source/i18n/units_converter.cpp b/icu4c/source/i18n/units_converter.cpp
index 3463387..a777d02 100644
--- a/icu4c/source/i18n/units_converter.cpp
+++ b/icu4c/source/i18n/units_converter.cpp
@@ -510,15 +510,36 @@
result -= conversionRate_.targetOffset; // Set the result to its index.
- if (result == 0)
- return 0.0; // If the result is zero, it does not matter if the conversion are reciprocal or not.
if (conversionRate_.reciprocal) {
+ if (result == 0) {
+ // TODO: demonstrate the resulting behaviour in tests... and figure
+ // out desired behaviour. (Theoretical result should be infinity,
+ // not 0.)
+ return 0.0;
+ }
result = 1.0 / result;
}
return result;
}
+double UnitConverter::convertInverse(double inputValue) const {
+ double result = inputValue;
+ if (conversionRate_.reciprocal) {
+ if (result == 0) {
+ // TODO: demonstrate the resulting behaviour in tests... and figure
+ // out desired behaviour. (Theoretical result should be infinity,
+ // not 0.)
+ return 0.0;
+ }
+ result = 1.0 / result;
+ }
+ result += conversionRate_.targetOffset;
+ result *= conversionRate_.factorDen / conversionRate_.factorNum;
+ result -= conversionRate_.sourceOffset;
+ return result;
+}
+
} // namespace units
U_NAMESPACE_END
diff --git a/icu4c/source/i18n/units_converter.h b/icu4c/source/i18n/units_converter.h
index 2f5dad8..8676405 100644
--- a/icu4c/source/i18n/units_converter.h
+++ b/icu4c/source/i18n/units_converter.h
@@ -144,14 +144,23 @@
const ConversionRates &ratesInfo, UErrorCode &status);
/**
- * Convert a value in the source unit to another value in the target unit.
+ * Convert a measurement expressed in the source unit to a measurement
+ * expressed in the target unit.
*
- * @param input_value the value that needs to be converted.
- * @param output_value the value that holds the result of the conversion.
- * @param status
+ * @param inputValue the value to be converted.
+ * @return the converted value.
*/
double convert(double inputValue) const;
+ /**
+ * The inverse of convert(): convert a measurement expressed in the target
+ * unit to a measurement expressed in the source unit.
+ *
+ * @param inputValue the value to be converted.
+ * @return the converted value.
+ */
+ double convertInverse(double inputValue) const;
+
private:
ConversionRate conversionRate_;
};
diff --git a/icu4c/source/i18n/units_router.cpp b/icu4c/source/i18n/units_router.cpp
index 759ae2b..9489a8d 100644
--- a/icu4c/source/i18n/units_router.cpp
+++ b/icu4c/source/i18n/units_router.cpp
@@ -10,6 +10,7 @@
#include "cstring.h"
#include "measunit_impl.h"
#include "number_decimalquantity.h"
+#include "number_roundingutils.h"
#include "resource.h"
#include "unicode/measure.h"
#include "units_data.h"
@@ -18,6 +19,29 @@
U_NAMESPACE_BEGIN
namespace units {
+using number::Precision;
+using number::impl::parseIncrementOption;
+
+Precision UnitsRouter::parseSkeletonToPrecision(icu::UnicodeString precisionSkeleton,
+ UErrorCode &status) {
+ if (U_FAILURE(status)) {
+ // As a member of UsagePrefsHandler, which is a friend of Precision, we
+ // get access to the default constructor.
+ return {};
+ }
+ constexpr int32_t kSkelPrefixLen = 20;
+ if (!precisionSkeleton.startsWith(UNICODE_STRING_SIMPLE("precision-increment/"))) {
+ status = U_INVALID_FORMAT_ERROR;
+ return {};
+ }
+ U_ASSERT(precisionSkeleton[kSkelPrefixLen - 1] == u'/');
+ StringSegment segment(precisionSkeleton, false);
+ segment.adjustOffset(kSkelPrefixLen);
+ Precision result;
+ parseIncrementOption(segment, result, status);
+ return result;
+}
+
UnitsRouter::UnitsRouter(MeasureUnit inputUnit, StringPiece region, StringPiece usage,
UErrorCode &status) {
// TODO: do we want to pass in ConversionRates and UnitPreferences instead
@@ -67,22 +91,32 @@
}
}
-RouteResult UnitsRouter::route(double quantity, UErrorCode &status) const {
- for (int i = 0, n = converterPreferences_.length(); i < n; i++) {
- const auto &converterPreference = *converterPreferences_[i];
- if (converterPreference.converter.greaterThanOrEqual(quantity * (1 + DBL_EPSILON),
- converterPreference.limit)) {
- return RouteResult(converterPreference.converter.convert(quantity, status),
- converterPreference.precision,
- converterPreference.targetUnit.copy(status));
+RouteResult UnitsRouter::route(double quantity, icu::number::impl::RoundingImpl *rounder, UErrorCode &status) const {
+ // Find the matching preference
+ const ConverterPreference *converterPreference = nullptr;
+ for (int32_t i = 0, n = converterPreferences_.length(); i < n; i++) {
+ converterPreference = converterPreferences_[i];
+ if (converterPreference->converter.greaterThanOrEqual(quantity * (1 + DBL_EPSILON),
+ converterPreference->limit)) {
+ break;
+ }
+ }
+ U_ASSERT(converterPreference != nullptr);
+
+ // Set up the rounder for this preference's precision
+ if (rounder != nullptr && rounder->fPrecision.isBogus()) {
+ if (converterPreference->precision.length() > 0) {
+ rounder->fPrecision = parseSkeletonToPrecision(converterPreference->precision, status);
+ } else {
+ // We use the same rounding mode as COMPACT notation: known to be a
+ // human-friendly rounding mode: integers, but add a decimal digit
+ // as needed to ensure we have at least 2 significant digits.
+ rounder->fPrecision = Precision::integer().withMinDigits(2);
}
}
- // In case of the `quantity` does not fit in any converter limit, use the last converter.
- const auto &lastConverterPreference = (*converterPreferences_[converterPreferences_.length() - 1]);
- return RouteResult(lastConverterPreference.converter.convert(quantity, status),
- lastConverterPreference.precision,
- lastConverterPreference.targetUnit.copy(status));
+ return RouteResult(converterPreference->converter.convert(quantity, rounder, status),
+ converterPreference->targetUnit.copy(status));
}
const MaybeStackVector<MeasureUnit> *UnitsRouter::getOutputUnits() const {
diff --git a/icu4c/source/i18n/units_router.h b/icu4c/source/i18n/units_router.h
index e6b74aa..bd7a93d 100644
--- a/icu4c/source/i18n/units_router.h
+++ b/icu4c/source/i18n/units_router.h
@@ -21,6 +21,9 @@
// Forward declarations
class Measure;
+namespace number {
+class Precision;
+}
namespace units {
@@ -31,19 +34,13 @@
// TODO(icu-units/icu#21): figure out the right mixed unit API.
MaybeStackVector<Measure> measures;
- // A skeleton string starting with a precision-increment.
- //
- // TODO(hugovdm): generalise? or narrow down to only a precision-increment?
- // or document that other skeleton elements are ignored?
- UnicodeString precision;
-
// The output unit for this RouteResult. This may be a MIXED unit - for
// example: "yard-and-foot-and-inch", for which `measures` will have three
// elements.
MeasureUnitImpl outputUnit;
- RouteResult(MaybeStackVector<Measure> measures, UnicodeString precision, MeasureUnitImpl outputUnit)
- : measures(std::move(measures)), precision(std::move(precision)), outputUnit(std::move(outputUnit)) {}
+ RouteResult(MaybeStackVector<Measure> measures, MeasureUnitImpl outputUnit)
+ : measures(std::move(measures)), outputUnit(std::move(outputUnit)) {}
};
/**
@@ -125,7 +122,16 @@
public:
UnitsRouter(MeasureUnit inputUnit, StringPiece locale, StringPiece usage, UErrorCode &status);
- RouteResult route(double quantity, UErrorCode &status) const;
+ /**
+ * Performs locale and usage sensitive unit conversion.
+ * @param quantity The quantity to convert, expressed in terms of inputUnit.
+ * @param rounder If not null, this RoundingImpl will be used to do rounding
+ * on the converted value. If the rounder lacks an fPrecision, the
+ * rounder will be modified to use the preferred precision for the usage
+ * and locale preference, alternatively with the default precision.
+ * @param status Receives status.
+ */
+ RouteResult route(double quantity, icu::number::impl::RoundingImpl *rounder, UErrorCode &status) const;
/**
* Returns the list of possible output units, i.e. the full set of
@@ -143,6 +149,9 @@
MaybeStackVector<MeasureUnit> outputUnits_;
MaybeStackVector<ConverterPreference> converterPreferences_;
+
+ static number::Precision parseSkeletonToPrecision(icu::UnicodeString precisionSkeleton,
+ UErrorCode &status);
};
} // namespace units
diff --git a/icu4c/source/test/depstest/dependencies.txt b/icu4c/source/test/depstest/dependencies.txt
index fda37de..a3d059a 100644
--- a/icu4c/source/test/depstest/dependencies.txt
+++ b/icu4c/source/test/depstest/dependencies.txt
@@ -1110,6 +1110,7 @@
units_data.o units_converter.o units_complexconverter.o units_router.o
deps
resourcebundle units_extra double_conversion number_representation formattable sort
+ number_rounding
group: decnumber
decContext.o decNumber.o
diff --git a/icu4c/source/test/intltest/numbertest_api.cpp b/icu4c/source/test/intltest/numbertest_api.cpp
index ecc0dca..ca2170d 100644
--- a/icu4c/source/test/intltest/numbertest_api.cpp
+++ b/icu4c/source/test/intltest/numbertest_api.cpp
@@ -753,21 +753,17 @@
4.28571,
u"4 metric tons, 285 kilograms, 710 grams");
-// // TODO(icu-units#73): deal with this "1 foot 12 inches" problem.
-// // At the time of writing, this test would pass, but is commented out
-// // because it reflects undesired behaviour:
-// assertFormatSingle(
-// u"Demonstrating the \"1 foot 12 inches\" problem",
-// nullptr,
-// u"unit/foot-and-inch",
-// NumberFormatter::with()
-// .unit(MeasureUnit::forIdentifier("foot-and-inch", status))
-// .precision(Precision::maxSignificantDigits(4))
-// .unitWidth(UNUM_UNIT_WIDTH_FULL_NAME),
-// Locale("en-US"),
-// 1.9999,
-// // This is undesireable but current behaviour:
-// u"1 foot, 12 inches");
+ assertFormatSingle(
+ u"Testing \"1 foot 12 inches\"",
+ nullptr,
+ u"unit/foot-and-inch",
+ NumberFormatter::with()
+ .unit(MeasureUnit::forIdentifier("foot-and-inch", status))
+ .precision(Precision::maxSignificantDigits(4))
+ .unitWidth(UNUM_UNIT_WIDTH_FULL_NAME),
+ Locale("en-US"),
+ 1.9999,
+ u"2 feet, 0 inches");
}
void NumberFormatterApiTest::unitCompoundMeasure() {
@@ -1161,6 +1157,10 @@
Locale("en-ZA"),
30500,
u"350 m");
+
+ // TODO(icu-units#38): improve unit testing coverage. E.g. add vehicle-fuel
+ // triggering inversion conversion code. Test with 0 too, to see
+ // divide-by-zero behaviour.
}
void NumberFormatterApiTest::unitUsageErrorCodes() {
diff --git a/icu4c/source/test/intltest/units_test.cpp b/icu4c/source/test/intltest/units_test.cpp
index 8053f1d..1132697 100644
--- a/icu4c/source/test/intltest/units_test.cpp
+++ b/icu4c/source/test/intltest/units_test.cpp
@@ -408,6 +408,10 @@
msg.clear();
msg.append("Converting 1000 ", status).append(x, status).append(" to ", status).append(y, status);
unitsTest->assertEqualsNear(msg.data(), expected, got, 0.0001 * expected);
+ double inverted = converter.convertInverse(got);
+ msg.clear();
+ msg.append("Converting back to ", status).append(x, status).append(" from ", status).append(y, status);
+ unitsTest->assertEqualsNear(msg.data(), 1000, inverted, 0.0001);
}
/**
@@ -439,7 +443,7 @@
}
void UnitsTest::testComplexUnitsConverter() {
- IcuTestErrorCode status(*this, "UnitsTest::testComplexUnitConversions");
+ IcuTestErrorCode status(*this, "UnitsTest::testComplexUnitsConverter");
ConversionRates rates(status);
MeasureUnit input = MeasureUnit::getFoot();
MeasureUnit output = MeasureUnit::forIdentifier("foot-and-inch", status);
@@ -449,7 +453,7 @@
auto converter = ComplexUnitsConverter(inputImpl, outputImpl, rates, status);
// Significantly less than 2.0.
- MaybeStackVector<Measure> measures = converter.convert(1.9999, status);
+ MaybeStackVector<Measure> measures = converter.convert(1.9999, nullptr, status);
assertEquals("measures length", 2, measures.length());
assertEquals("1.9999: measures[0] value", 1.0, measures[0]->getNumber().getDouble(status));
assertEquals("1.9999: measures[0] unit", MeasureUnit::getFoot().getIdentifier(),
@@ -458,13 +462,13 @@
assertEquals("1.9999: measures[1] unit", MeasureUnit::getInch().getIdentifier(),
measures[1]->getUnit().getIdentifier());
- // TODO: consider factoring out the set of tests to make this function more
+ // TODO(icu-units#100): consider factoring out the set of tests to make this function more
// data-driven, *after* dealing appropriately with the memory leaks that can
// be demonstrated by this code.
- // TODO: reusing measures results in a leak.
+ // TODO(icu-units#100): reusing measures results in a leak.
// A minimal nudge under 2.0.
- MaybeStackVector<Measure> measures2 = converter.convert((2.0 - DBL_EPSILON), status);
+ MaybeStackVector<Measure> measures2 = converter.convert((2.0 - DBL_EPSILON), nullptr, status);
assertEquals("measures length", 2, measures2.length());
assertEquals("1 - eps: measures[0] value", 2.0, measures2[0]->getNumber().getDouble(status));
assertEquals("1 - eps: measures[0] unit", MeasureUnit::getFoot().getIdentifier(),
@@ -480,14 +484,14 @@
// An epsilon's nudge under one light-year: should give 1 ly, 0 m.
input = MeasureUnit::getLightYear();
output = MeasureUnit::forIdentifier("light-year-and-meter", status);
- // TODO: reusing tempInput and tempOutput results in a leak.
+ // TODO(icu-units#100): reusing tempInput and tempOutput results in a leak.
MeasureUnitImpl tempInput3, tempOutput3;
const MeasureUnitImpl &inputImpl3 = MeasureUnitImpl::forMeasureUnit(input, tempInput3, status);
const MeasureUnitImpl &outputImpl3 = MeasureUnitImpl::forMeasureUnit(output, tempOutput3, status);
- // TODO: reusing converter results in a leak.
+ // TODO(icu-units#100): reusing converter results in a leak.
ComplexUnitsConverter converter3 = ComplexUnitsConverter(inputImpl3, outputImpl3, rates, status);
- // TODO: reusing measures results in a leak.
- MaybeStackVector<Measure> measures3 = converter3.convert((2.0 - DBL_EPSILON), status);
+ // TODO(icu-units#100): reusing measures results in a leak.
+ MaybeStackVector<Measure> measures3 = converter3.convert((2.0 - DBL_EPSILON), nullptr, status);
assertEquals("measures length", 2, measures3.length());
assertEquals("light-year test: measures[0] value", 2.0, measures3[0]->getNumber().getDouble(status));
assertEquals("light-year test: measures[0] unit", MeasureUnit::getLightYear().getIdentifier(),
@@ -499,7 +503,7 @@
// 1e-15 light years is 9.46073 meters (calculated using "bc" and the CLDR
// conversion factor). With double-precision maths, we get 10.5. In this
// case, we're off by almost 1 meter.
- MaybeStackVector<Measure> measures4 = converter3.convert((1.0 + 1e-15), status);
+ MaybeStackVector<Measure> measures4 = converter3.convert((1.0 + 1e-15), nullptr, status);
assertEquals("measures length", 2, measures4.length());
assertEquals("light-year test: measures[0] value", 1.0, measures4[0]->getNumber().getDouble(status));
assertEquals("light-year test: measures[0] unit", MeasureUnit::getLightYear().getIdentifier(),
@@ -511,7 +515,7 @@
// 2e-16 light years is 1.892146 meters. We consider this in the noise, and
// thus expect a 0. (This test fails when 2e-16 is increased to 4e-16.)
- MaybeStackVector<Measure> measures5 = converter3.convert((1.0 + 2e-16), status);
+ MaybeStackVector<Measure> measures5 = converter3.convert((1.0 + 2e-16), nullptr, status);
assertEquals("measures length", 2, measures5.length());
assertEquals("light-year test: measures[0] value", 1.0, measures5[0]->getNumber().getDouble(status));
assertEquals("light-year test: measures[0] unit", MeasureUnit::getLightYear().getIdentifier(),
@@ -532,7 +536,7 @@
ConversionRates conversionRates(status);
ComplexUnitsConverter complexConverter(source, target, conversionRates, status);
- auto measures = complexConverter.convert(10.0, status);
+ auto measures = complexConverter.convert(10.0, nullptr, status);
U_ASSERT(measures.length() == 2);
assertEquals("inch-and-foot unit 0", "inch", measures[0]->getUnit().getIdentifier());
@@ -732,7 +736,7 @@
if (status.errIfFailureAndReset("Failure before router.route")) {
return;
}
- auto routeResult = router.route(inputAmount, status);
+ RouteResult routeResult = router.route(inputAmount, nullptr, status);
if (status.errIfFailureAndReset("router.route(inputAmount, ...)")) {
return;
}
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UnitConversionHandler.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UnitConversionHandler.java
index db27059..0612c63 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UnitConversionHandler.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UnitConversionHandler.java
@@ -8,7 +8,6 @@
import com.ibm.icu.util.Measure;
import com.ibm.icu.util.MeasureUnit;
-import java.util.ArrayList;
import java.util.List;
/**
@@ -49,7 +48,7 @@
MicroProps result = this.fParent.processQuantity(quantity);
quantity.roundToInfinity(); // Enables toDouble
- List<Measure> measures = this.fComplexUnitConverter.convert(quantity.toBigDecimal());
+ List<Measure> measures = this.fComplexUnitConverter.convert(quantity.toBigDecimal(), result.rounder);
result.outputUnit = this.fOutputUnit;
UsagePrefsHandler.mixedMeasuresToMicros(measures, quantity, result);
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UsagePrefsHandler.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UsagePrefsHandler.java
index 74c13b6..ca20045 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UsagePrefsHandler.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UsagePrefsHandler.java
@@ -2,16 +2,13 @@
// License & terms of use: http://www.unicode.org/copyright.html
package com.ibm.icu.impl.number;
-import com.ibm.icu.impl.IllegalIcuArgumentException;
import com.ibm.icu.impl.units.MeasureUnitImpl;
import com.ibm.icu.impl.units.UnitsRouter;
-import com.ibm.icu.number.Precision;
import com.ibm.icu.util.Measure;
import com.ibm.icu.util.MeasureUnit;
import com.ibm.icu.util.ULocale;
import java.math.BigDecimal;
-import java.math.MathContext;
import java.util.ArrayList;
import java.util.List;
@@ -28,46 +25,29 @@
new UnitsRouter(MeasureUnitImpl.forIdentifier(inputUnit.getIdentifier()), locale.getCountry(), usage);
}
- private static Precision parseSkeletonToPrecision(String precisionSkeleton) {
- final String kSuffixPrefix = "precision-increment/";
- if (!precisionSkeleton.startsWith(kSuffixPrefix)) {
- throw new IllegalIcuArgumentException("precisionSkeleton is only precision-increment");
- }
-
- String skeleton = precisionSkeleton.substring(kSuffixPrefix.length());
- String skeletons[] = skeleton.split("/");
- BigDecimal num = new BigDecimal(skeletons[0]);
- BigDecimal den =
- skeletons.length == 2 ?
- new BigDecimal(skeletons[1]) :
- new BigDecimal("1");
-
-
- return Precision.increment(num.divide(den, MathContext.DECIMAL128));
- }
-
/**
* Populates micros.mixedMeasures and modifies quantity, based on the values
* in measures.
*/
- protected static void mixedMeasuresToMicros(List<Measure> measures, DecimalQuantity quantity, MicroProps micros) {
- micros.mixedMeasures = new ArrayList<>();
+ protected static void
+ mixedMeasuresToMicros(List<Measure> measures, DecimalQuantity outQuantity, MicroProps outMicros) {
+ outMicros.mixedMeasures = new ArrayList<>();
if (measures.size() > 1) {
// For debugging
- assert (micros.outputUnit.getComplexity() == MeasureUnit.Complexity.MIXED);
+ assert (outMicros.outputUnit.getComplexity() == MeasureUnit.Complexity.MIXED);
// Check that we received the expected number of measurements:
- assert measures.size() == micros.outputUnit.splitToSingleUnits().size();
+ assert measures.size() == outMicros.outputUnit.splitToSingleUnits().size();
// Mixed units: except for the last value, we pass all values to the
// LongNameHandler via micros->mixedMeasures.
for (int i = 0, n = measures.size() - 1; i < n; i++) {
- micros.mixedMeasures.add(measures.get(i));
+ outMicros.mixedMeasures.add(measures.get(i));
}
}
// The last value (potentially the only value) gets passed on via quantity.
- quantity.setToBigDecimal((BigDecimal) measures.get(measures.size()- 1).getNumber());
+ outQuantity.setToBigDecimal((BigDecimal) measures.get(measures.size()- 1).getNumber());
}
/**
@@ -93,29 +73,12 @@
MicroProps micros = this.fParent.processQuantity(quantity);
quantity.roundToInfinity(); // Enables toDouble
- final UnitsRouter.RouteResult routed = fUnitsRouter.route(quantity.toBigDecimal());
+ final UnitsRouter.RouteResult routed = fUnitsRouter.route(quantity.toBigDecimal(), micros);
final List<Measure> routedMeasures = routed.measures;
micros.outputUnit = routed.outputUnit.build();
UsagePrefsHandler.mixedMeasuresToMicros(routedMeasures, quantity, micros);
-
- String precisionSkeleton = routed.precision;
-
- assert micros.rounder != null;
-
- if (micros.rounder instanceof Precision.BogusRounder) {
- Precision.BogusRounder rounder = (Precision.BogusRounder)micros.rounder;
- if (precisionSkeleton != null && precisionSkeleton.length() > 0) {
- micros.rounder = rounder.into(parseSkeletonToPrecision(precisionSkeleton));
- } else {
- // We use the same rounding mode as COMPACT notation: known to be a
- // human-friendly rounding mode: integers, but add a decimal digit
- // as needed to ensure we have at least 2 significant digits.
- micros.rounder = rounder.into(Precision.integer().withMinDigits(2));
- }
- }
-
return micros;
}
}
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/ComplexUnitsConverter.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/ComplexUnitsConverter.java
index aec0e5f..d864775 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/ComplexUnitsConverter.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/ComplexUnitsConverter.java
@@ -1,11 +1,12 @@
// © 2020 and later: Unicode, Inc. and others.
// License & terms of use: http://www.unicode.org/copyright.html
-
-
package com.ibm.icu.impl.units;
-
+import com.ibm.icu.impl.number.DecimalQuantity;
+import com.ibm.icu.impl.number.DecimalQuantity_DualStorageBCD;
+import com.ibm.icu.number.Precision;
import com.ibm.icu.util.Measure;
+import com.ibm.icu.util.MeasureUnit;
import java.math.BigDecimal;
import java.math.RoundingMode;
@@ -26,7 +27,10 @@
public static final BigDecimal EPSILON = BigDecimal.valueOf(Math.ulp(1.0));
public static final BigDecimal EPSILON_MULTIPLIER = BigDecimal.valueOf(1).add(EPSILON);
private ArrayList<UnitConverter> unitConverters_;
+ // Individual units of mixed units, sorted big to small
private ArrayList<MeasureUnitImpl> units_;
+ // Individual units of mixed units, sorted in desired output order
+ private ArrayList<MeasureUnit> outputUnits_;
/**
* Constructor of `ComplexUnitsConverter`.
@@ -40,6 +44,10 @@
public ComplexUnitsConverter(MeasureUnitImpl inputUnit, MeasureUnitImpl outputUnits,
ConversionRates conversionRates) {
units_ = outputUnits.extractIndividualUnits();
+ outputUnits_ = new ArrayList<>(units_.size());
+ for (MeasureUnitImpl itr : units_) {
+ outputUnits_.add(itr.build());
+ }
assert (!units_.isEmpty());
// Sort the units in a descending order.
@@ -95,8 +103,16 @@
* the smallest element is the only element that could have fractional values. And all
* other elements are floored to the nearest integer
*/
- public List<Measure> convert(BigDecimal quantity) {
- List<Measure> result = new ArrayList<>();
+ public List<Measure> convert(BigDecimal quantity, Precision rounder) {
+ List<Measure> result = new ArrayList<>(unitConverters_.size());
+
+ // For N converters:
+ // - the first converter converts from the input unit to the largest
+ // unit,
+ // - N-1 converters convert to bigger units for which we want integers,
+ // - the Nth converter (index N-1) converts to the smallest unit, which
+ // isn't (necessarily) an integer.
+ List<BigDecimal> intValues = new ArrayList<>(unitConverters_.size() - 1);
for (int i = 0, n = unitConverters_.size(); i < n; ++i) {
quantity = (unitConverters_.get(i)).convert(quantity);
@@ -108,21 +124,85 @@
// decision is made. However after the thresholding, we use the
// original values to ensure unbiased accuracy (to the extent of
// double's capabilities).
- BigDecimal newQuantity = quantity.multiply(EPSILON_MULTIPLIER).setScale(0, RoundingMode.FLOOR);
-
- result.add(new Measure(newQuantity, units_.get(i).build()));
+ BigDecimal roundedQuantity =
+ quantity.multiply(EPSILON_MULTIPLIER).setScale(0, RoundingMode.FLOOR);
+ intValues.add(roundedQuantity);
// Keep the residual of the quantity.
// For example: `3.6 feet`, keep only `0.6 feet`
- quantity = quantity.subtract(newQuantity);
+ quantity = quantity.subtract(roundedQuantity);
if (quantity.compareTo(BigDecimal.ZERO) == -1) {
quantity = BigDecimal.ZERO;
}
} else { // LAST ELEMENT
+ if (rounder == null) {
+ // Nothing to do for the last element.
+ break;
+ }
+
+ // Round the last value
+ // TODO(ICU-21288): get smarter about precision for mixed units.
+ DecimalQuantity quant = new DecimalQuantity_DualStorageBCD(quantity);
+ rounder.apply(quant);
+ quantity = quant.toBigDecimal();
+ if (i == 0) {
+ // Last element is also the first element, so we're done
+ break;
+ }
+
+ // Check if there's a carry, and bubble it back up the resulting intValues.
+ BigDecimal carry = unitConverters_.get(i)
+ .convertInverse(quantity)
+ .multiply(EPSILON_MULTIPLIER)
+ .setScale(0, RoundingMode.FLOOR);
+ if (carry.compareTo(BigDecimal.ZERO) <= 0) { // carry is not greater than zero
+ break;
+ }
+ quantity = quantity.subtract(unitConverters_.get(i).convert(carry));
+ intValues.set(i - 1, intValues.get(i - 1).add(carry));
+
+ // We don't use the first converter: that one is for the input unit
+ for (int j = i - 1; j > 0; j--) {
+ carry = unitConverters_.get(j)
+ .convertInverse(intValues.get(j))
+ .multiply(EPSILON_MULTIPLIER)
+ .setScale(0, RoundingMode.FLOOR);
+ if (carry.compareTo(BigDecimal.ZERO) <= 0) { // carry is not greater than zero
+ break;
+ }
+ intValues.set(j, intValues.get(j).subtract(unitConverters_.get(j).convert(carry)));
+ intValues.set(j - 1, intValues.get(j - 1).add(carry));
+ }
+ }
+ }
+
+ // Package values into Measure instances in result:
+ for (int i = 0, n = unitConverters_.size(); i < n; ++i) {
+ if (i < n - 1) {
+ result.add(new Measure(intValues.get(i), units_.get(i).build()));
+ } else {
result.add(new Measure(quantity, units_.get(i).build()));
}
}
+ for (int i = 0; i < result.size(); i++) {
+ for (int j = i; j < result.size(); j++) {
+ // Find the next expected unit, and swap it into place.
+ if (result.get(j).getUnit().equals(outputUnits_.get(i))) {
+ if (j != i) {
+ Measure tmp = result.get(j);
+ result.set(j, result.get(i));
+ result.set(i, tmp);
+ }
+ }
+ }
+ }
+
return result;
}
+
+ @Override
+ public String toString() {
+ return "ComplexUnitsConverter [unitConverters_=" + unitConverters_ + ", units_=" + units_ + "]";
+ }
}
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/MeasureUnitImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/MeasureUnitImpl.java
index ac8b1ed..d46487d 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/MeasureUnitImpl.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/MeasureUnitImpl.java
@@ -1,7 +1,5 @@
// © 2020 and later: Unicode, Inc. and others.
// License & terms of use: http://www.unicode.org/copyright.html
-
-
package com.ibm.icu.impl.units;
import com.ibm.icu.util.BytesTrie;
@@ -765,4 +763,9 @@
return o1.compareTo(o2);
}
}
-}
\ No newline at end of file
+
+ @Override
+ public String toString() {
+ return "MeasureUnitImpl [" + build().getIdentifier() + "]";
+ }
+}
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitConverter.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitConverter.java
index 9ac7e1c..7a38852 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitConverter.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitConverter.java
@@ -1,7 +1,5 @@
// © 2020 and later: Unicode, Inc. and others.
// License & terms of use: http://www.unicode.org/copyright.html
-
-
package com.ibm.icu.impl.units;
import com.ibm.icu.util.MeasureUnit;
@@ -88,6 +86,10 @@
return inputValue.multiply(this.conversionRate).add(offset);
}
+ public BigDecimal convertInverse(BigDecimal inputValue) {
+ return inputValue.subtract(offset).divide(this.conversionRate, DECIMAL128);
+ }
+
public enum Convertibility {
CONVERTIBLE,
RECIPROCAL,
@@ -308,4 +310,9 @@
}
}
}
+
+ @Override
+ public String toString() {
+ return "UnitConverter [conversionRate=" + conversionRate + ", offset=" + offset + "]";
+ }
}
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitPreferences.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitPreferences.java
index ac5d9a7..5dedeeb 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitPreferences.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitPreferences.java
@@ -1,7 +1,5 @@
// © 2020 and later: Unicode, Inc. and others.
// License & terms of use: http://www.unicode.org/copyright.html
-
-
package com.ibm.icu.impl.units;
import com.ibm.icu.impl.ICUData;
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitsRouter.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitsRouter.java
index 4f8fbb6..f2ebfda 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitsRouter.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitsRouter.java
@@ -1,9 +1,10 @@
// © 2020 and later: Unicode, Inc. and others.
// License & terms of use: http://www.unicode.org/copyright.html
-
-
package com.ibm.icu.impl.units;
+import com.ibm.icu.impl.IllegalIcuArgumentException;
+import com.ibm.icu.impl.number.MicroProps;
+import com.ibm.icu.number.Precision;
import com.ibm.icu.util.Measure;
import com.ibm.icu.util.MeasureUnit;
@@ -79,27 +80,55 @@
}
}
- public RouteResult route(BigDecimal quantity) {
- for (ConverterPreference converterPreference :
- converterPreferences_) {
+ /** If micros.rounder is a BogusRounder, this function replaces it with a valid one. */
+ public RouteResult route(BigDecimal quantity, MicroProps micros) {
+ Precision rounder = micros == null ? null : micros.rounder;
+ ConverterPreference converterPreference = null;
+ for (ConverterPreference itr : converterPreferences_) {
+ converterPreference = itr;
if (converterPreference.converter.greaterThanOrEqual(quantity, converterPreference.limit)) {
- return new RouteResult(
- converterPreference.converter.convert(quantity),
- converterPreference.precision,
- converterPreference.targetUnit
- );
+ break;
+ }
+ }
+ assert converterPreference != null;
+ assert converterPreference.precision != null;
+
+ // Set up the rounder for this preference's precision
+ if (rounder != null && rounder instanceof Precision.BogusRounder) {
+ Precision.BogusRounder bogus = (Precision.BogusRounder)rounder;
+ if (converterPreference.precision.length() > 0) {
+ rounder = bogus.into(parseSkeletonToPrecision(converterPreference.precision));
+ } else {
+ // We use the same rounding mode as COMPACT notation: known to be a
+ // human-friendly rounding mode: integers, but add a decimal digit
+ // as needed to ensure we have at least 2 significant digits.
+ rounder = bogus.into(Precision.integer().withMinDigits(2));
}
}
- // In case of the `quantity` does not fit in any converter limit, use the last converter.
- ConverterPreference lastConverterPreference = converterPreferences_.get(converterPreferences_.size() - 1);
+ if (micros != null) {
+ micros.rounder = rounder;
+ }
return new RouteResult(
- lastConverterPreference.converter.convert(quantity),
- lastConverterPreference.precision,
- lastConverterPreference.targetUnit
+ converterPreference.converter.convert(quantity, rounder),
+ converterPreference.targetUnit
);
}
+ private static Precision parseSkeletonToPrecision(String precisionSkeleton) {
+ final String kSkeletonPrefix = "precision-increment/";
+ if (!precisionSkeleton.startsWith(kSkeletonPrefix)) {
+ throw new IllegalIcuArgumentException("precisionSkeleton is only precision-increment");
+ }
+
+ // TODO(icu-units#104): the C++ code uses a more sophisticated
+ // parseIncrementOption which supports "withMinFraction" - e.g.
+ // "precision-increment/0.5". Test with a unit preference that uses
+ // this, and fix Java.
+ String incrementValue = precisionSkeleton.substring(kSkeletonPrefix.length());
+ return Precision.increment(new BigDecimal(incrementValue));
+ }
+
/**
* Returns the list of possible output units, i.e. the full set of
* preferences, for the localized, usage-specific unit preferences.
@@ -152,20 +181,13 @@
// TODO(icu-units/icu#21): figure out the right mixed unit API.
public final List<Measure> measures;
- // A skeleton string starting with a precision-increment.
- //
- // TODO(hugovdm): generalise? or narrow down to only a precision-increment?
- // or document that other skeleton elements are ignored?
- public final String precision;
-
// The output unit for this RouteResult. This may be a MIXED unit - for
// example: "yard-and-foot-and-inch", for which `measures` will have three
// elements.
public final MeasureUnitImpl outputUnit;
- RouteResult(List<Measure> measures, String precision, MeasureUnitImpl outputUnit) {
+ RouteResult(List<Measure> measures, MeasureUnitImpl outputUnit) {
this.measures = measures;
- this.precision = precision;
this.outputUnit = outputUnit;
}
}
diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/impl/UnitsTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/impl/UnitsTest.java
index 3ace927..69df74d 100644
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/impl/UnitsTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/impl/UnitsTest.java
@@ -1,7 +1,5 @@
// © 2020 and later: Unicode, Inc. and others.
// License & terms of use: http://www.unicode.org/copyright.html
-
-
package com.ibm.icu.dev.test.impl;
import com.ibm.icu.dev.test.TestUtil;
@@ -47,7 +45,7 @@
ComplexUnitsConverter converter = new ComplexUnitsConverter(inputImpl, outputImpl, rates);
// Significantly less than 2.0.
- List<Measure> measures = converter.convert(BigDecimal.valueOf(1.9999));
+ List<Measure> measures = converter.convert(BigDecimal.valueOf(1.9999), null);
assertEquals("measures length", 2, measures.size());
assertEquals("1.9999: measures[0] value", BigDecimal.valueOf(1), measures.get(0).getNumber());
assertEquals("1.9999: measures[0] unit", MeasureUnit.FOOT.getIdentifier(),
@@ -58,13 +56,14 @@
assertEquals("1.9999: measures[1] unit", MeasureUnit.INCH.getIdentifier(),
measures.get(1).getUnit().getIdentifier());
- // TODO: consider factoring out the set of tests to make this function more
- // data-driven, *after* dealing appropriately with the memory leaks that can
- // be demonstrated by this code.
+ // TODO(icu-units#100): consider factoring out the set of tests to make
+ // this function more data-driven, *after* dealing appropriately with
+ // the C++ memory leaks that can be demonstrated by the C++ version of
+ // this code.
- // TODO: reusing measures results in a leak.
// A minimal nudge under 2.0.
- List<Measure> measures2 = converter.convert(BigDecimal.valueOf(2.0).subtract(ComplexUnitsConverter.EPSILON));
+ List<Measure> measures2 =
+ converter.convert(BigDecimal.valueOf(2.0).subtract(ComplexUnitsConverter.EPSILON), null);
assertEquals("measures length", 2, measures2.size());
assertEquals("1 - eps: measures[0] value", BigDecimal.valueOf(2), measures2.get(0).getNumber());
assertEquals("1 - eps: measures[0] unit", MeasureUnit.FOOT.getIdentifier(),
@@ -83,11 +82,10 @@
final MeasureUnitImpl inputImpl3 = MeasureUnitImpl.forIdentifier(input.getIdentifier());
final MeasureUnitImpl outputImpl3 = MeasureUnitImpl.forIdentifier(output.getIdentifier());
- // TODO: reusing converter results in a leak.
ComplexUnitsConverter converter3 = new ComplexUnitsConverter(inputImpl3, outputImpl3, rates);
- // TODO: reusing measures results in a leak.
- List<Measure> measures3 = converter3.convert(BigDecimal.valueOf(2.0).subtract(ComplexUnitsConverter.EPSILON));
+ List<Measure> measures3 =
+ converter3.convert(BigDecimal.valueOf(2.0).subtract(ComplexUnitsConverter.EPSILON), null);
assertEquals("measures length", 2, measures3.size());
assertEquals("light-year test: measures[0] value", BigDecimal.valueOf(2), measures3.get(0).getNumber());
assertEquals("light-year test: measures[0] unit", MeasureUnit.LIGHT_YEAR.getIdentifier(),
@@ -99,7 +97,7 @@
// 1e-15 light years is 9.46073 meters (calculated using "bc" and the CLDR
// conversion factor). With double-precision maths, we get 10.5. In this
// case, we're off by almost 1 meter.
- List<Measure> measures4 = converter3.convert(BigDecimal.valueOf(1.0 + 1e-15));
+ List<Measure> measures4 = converter3.convert(BigDecimal.valueOf(1.0 + 1e-15), null);
assertEquals("measures length", 2, measures4.size());
assertEquals("light-year test: measures[0] value", BigDecimal.ONE, measures4.get(0).getNumber());
assertEquals("light-year test: measures[0] unit", MeasureUnit.LIGHT_YEAR.getIdentifier(),
@@ -112,7 +110,7 @@
// 2e-16 light years is 1.892146 meters. We consider this in the noise, and
// thus expect a 0. (This test fails when 2e-16 is increased to 4e-16.)
- List<Measure> measures5 = converter3.convert(BigDecimal.valueOf(1.0 + 2e-17));
+ List<Measure> measures5 = converter3.convert(BigDecimal.valueOf(1.0 + 2e-17), null);
assertEquals("measures length", 2, measures5.size());
assertEquals("light-year test: measures[0] value", BigDecimal.ONE, measures5.get(0).getNumber());
assertEquals("light-year test: measures[0] unit", MeasureUnit.LIGHT_YEAR.getIdentifier(),
@@ -134,14 +132,14 @@
ConversionRates conversionRates = new ConversionRates();
ComplexUnitsConverter complexConverter = new ComplexUnitsConverter(source, target, conversionRates);
- List<Measure> measures = complexConverter.convert(BigDecimal.valueOf(10.0));
+ List<Measure> measures = complexConverter.convert(BigDecimal.valueOf(10.0), null);
assertEquals(measures.size(), 2);
- assertEquals("inch-and-foot unit 0", "foot", measures.get(0).getUnit().getIdentifier());
- assertEquals("inch-and-foot unit 1", "inch", measures.get(1).getUnit().getIdentifier());
+ assertEquals("inch-and-foot unit 0", "inch", measures.get(0).getUnit().getIdentifier());
+ assertEquals("inch-and-foot unit 1", "foot", measures.get(1).getUnit().getIdentifier());
- assertTrue("inch-and-foot value 0", compareTwoBigDecimal(BigDecimal.valueOf(32), BigDecimal.valueOf(measures.get(0).getNumber().doubleValue()), BigDecimal.valueOf(0.0001)));
- assertTrue("inch-and-foot value 1", compareTwoBigDecimal(BigDecimal.valueOf(9.7008), BigDecimal.valueOf(measures.get(1).getNumber().doubleValue()), BigDecimal.valueOf(0.0001)));
+ assertEquals("inch-and-foot value 0", 9.7008, measures.get(0).getNumber().doubleValue(), 0.0001);
+ assertEquals("inch-and-foot value 1", 32, measures.get(1).getNumber().doubleValue(), 0.0001);
}
@@ -254,21 +252,36 @@
for (TestCase testCase :
tests) {
UnitConverter converter = new UnitConverter(testCase.source, testCase.target, conversionRates);
- if (compareTwoBigDecimal(testCase.expected, converter.convert(testCase.input), BigDecimal.valueOf(0.000001))) {
+ BigDecimal got = converter.convert(testCase.input);
+ if (compareTwoBigDecimal(testCase.expected, got, BigDecimal.valueOf(0.000001))) {
continue;
} else {
fail(new StringBuilder()
.append(testCase.category)
- .append(" ")
+ .append(": Converting 1000 ")
.append(testCase.sourceString)
- .append(" ")
+ .append(" to ")
.append(testCase.targetString)
- .append(" ")
- .append(converter.convert(testCase.input).toString())
- .append(" expected ")
+ .append(", got ")
+ .append(got)
+ .append(", expected ")
.append(testCase.expected.toString())
.toString());
}
+ BigDecimal inverted = converter.convertInverse(testCase.input);
+ if (compareTwoBigDecimal(BigDecimal.valueOf(1000), inverted, BigDecimal.valueOf(0.000001))) {
+ continue;
+ } else {
+ fail(new StringBuilder()
+ .append("Converting back to ")
+ .append(testCase.sourceString)
+ .append(" from ")
+ .append(testCase.targetString)
+ .append(": got ")
+ .append(inverted)
+ .append(", expected 1000")
+ .toString());
+ }
}
}
@@ -345,7 +358,7 @@
for (TestCase testCase :
tests) {
UnitsRouter router = new UnitsRouter(testCase.inputUnit.second, testCase.region, testCase.usage);
- List<Measure> measures = router.route(testCase.input).measures;
+ List<Measure> measures = router.route(testCase.input, null).measures;
assertEquals("Measures size must be the same as expected units",
measures.size(), testCase.expectedInOrder.size());
diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java
index 3bed7ba..22bd114 100644
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java
@@ -715,21 +715,17 @@
4.28571,
"4 metric tons, 285 kilograms, 710 grams");
-// // TODO(icu-units#73): deal with this "1 foot 12 inches" problem.
-// // At the time of writing, this test would pass, but is commented out
-// // because it reflects undesired behaviour:
-// assertFormatSingle(
-// u"Demonstrating the \"1 foot 12 inches\" problem",
-// nullptr,
-// u"unit/foot-and-inch",
-// NumberFormatter::with()
-// .unit(MeasureUnit::forIdentifier("foot-and-inch"))
-// .precision(Precision::maxSignificantDigits(4))
-// .unitWidth(UNUM_UNIT_WIDTH_FULL_NAME),
-// Locale("en-US"),
-// 1.9999,
-// // This is undesireable but current behaviour:
-// u"1 foot, 12 inches");
+ assertFormatSingle(
+ "Testing \"1 foot 12 inches\"",
+ null,
+ "unit/foot-and-inch",
+ NumberFormatter.with()
+ .unit(MeasureUnit.forIdentifier("foot-and-inch"))
+ .precision(Precision.maxSignificantDigits(4))
+ .unitWidth(UnitWidth.FULL_NAME),
+ new ULocale("en-US"),
+ 1.9999,
+ "2 feet, 0 inches");
}
@Test
@@ -1118,7 +1114,11 @@
new ULocale("en-ZA"),
30500,
"350 m");
-}
+
+ // TODO(icu-units#38): improve unit testing coverage. E.g. add
+ // vehicle-fuel triggering inversion conversion code. Test with 0 too,
+ // to see divide-by-zero behaviour.
+ }
@Test
