From aeccdbf85c781db2fde904e3d85d8e805a1fa4b9 Mon Sep 17 00:00:00 2001 From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Date: Wed, 6 May 2020 15:07:48 +0200 Subject: [PATCH] Implement slicing via `UnitPreference **&outPreferences`. (Still multiple TODOs for make things cleaner/neater.) --- icu4c/source/i18n/unitsdata.cpp | 29 +++++--------------- icu4c/source/i18n/unitsdata.h | 8 +++++- icu4c/source/test/intltest/unitsdatatest.cpp | 24 ++++++++++------ 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/icu4c/source/i18n/unitsdata.cpp b/icu4c/source/i18n/unitsdata.cpp index f56fda3dea5..d5d1532b357 100644 --- a/icu4c/source/i18n/unitsdata.cpp +++ b/icu4c/source/i18n/unitsdata.cpp @@ -358,32 +358,17 @@ U_I18N_API UnitPreferences::UnitPreferences(UErrorCode &status) { ures_getAllItemsWithFallback(unitsBundle.getAlias(), "unitPreferenceData", sink, status); } +// TODO/WIP: make outPrefernces const, make function const, propagate const as needed. +// TODO/WIP: create a simpler class to replace `UnitPreference **&outPrefrences`. void U_I18N_API UnitPreferences::getPreferencesFor(const char *category, const char *usage, - const char *region, - MaybeStackVector *outPreferences, - UErrorCode &status) { - // UnitPreferenceMetadata *m = getMetadata(category, usage, region); + const char *region, UnitPreference **&outPreferences, + int32_t &preferenceCount, UErrorCode &status) { int32_t idx = getPreferenceMetadataIndex(&metadata_, category, usage, region, status); if (U_FAILURE(status)) { return; } U_ASSERT(idx >= 0); // Failures should have been taken care of by `status`. - UnitPreferenceMetadata *m = metadata_[idx]; - for (int32_t pref = m->prefsOffset; pref < m->prefsOffset + m->prefsCount; pref++) { - UnitPreference *p = unitPrefs_[pref]; - // TODO(review): we're making a full copy of the preferences here. - // Considering UnitPreferences instances should simply stick around, we - // could also simply return pointers at these instances. What is the - // appropriate data structure (array/vector) for variable set of - // pointers? MaybeStackVector could probably work, but - // ugly as a double-dereference?) - UnitPreference *outP = outPreferences->emplaceBack(); - if (!outP) { - status = U_MEMORY_ALLOCATION_ERROR; - return; - } - outP->unit.copyFrom(p->unit, status); - outP->geq = p->geq; - outP->skeleton.copyFrom(p->skeleton, status); - } + const UnitPreferenceMetadata *m = metadata_[idx]; + outPreferences = unitPrefs_.getAlias() + m->prefsOffset; + preferenceCount = m->prefsCount; } U_NAMESPACE_END diff --git a/icu4c/source/i18n/unitsdata.h b/icu4c/source/i18n/unitsdata.h index 7c49d1689fd..8265b93b096 100644 --- a/icu4c/source/i18n/unitsdata.h +++ b/icu4c/source/i18n/unitsdata.h @@ -147,9 +147,15 @@ class U_I18N_API UnitPreferences { * falls back to region "001" ("world"). * @param outPreferences The vector to which preferences will be added. * @param status Receives status. + * + * - TODO/WIP: make outPrefernces const, make function const, propagate + * const as needed. + * - TODO/WIP: create a simpler class to replace `UnitPreference + * **&outPrefrences`. */ void getPreferencesFor(const char *category, const char *usage, const char *region, - MaybeStackVector *outPreferences, UErrorCode &status); + UnitPreference **&outPreferences, int32_t &preferenceCount, + UErrorCode &status); protected: // Metadata about the sets of preferences, this is the index for looking up diff --git a/icu4c/source/test/intltest/unitsdatatest.cpp b/icu4c/source/test/intltest/unitsdatatest.cpp index 12063d28771..dad34a16b63 100644 --- a/icu4c/source/test/intltest/unitsdatatest.cpp +++ b/icu4c/source/test/intltest/unitsdatatest.cpp @@ -89,28 +89,36 @@ void UnitsDataTest::testGetPreferences() { assertTrue(UnicodeString("Preferences count: ") + unitPrefs->length() + " > 250", unitPrefs->length() > 250); - // Dump all preferences... TODO: remove? This was just debugging/development output. + // Dump all preferences... TODO/WIP: remove whole block? This was just + // debugging/development output. logln("Unit Preferences:"); for (int32_t i = 0; i < metadata->length(); i++) { logln("%d: category %s, usage %s, region %s, offset %d, count %d", i, (*metadata)[i]->category.data(), (*metadata)[i]->usage.data(), (*metadata)[i]->region.data(), (*metadata)[i]->prefsOffset, (*metadata)[i]->prefsCount); - for (int32_t j = (*metadata)[i]->prefsOffset; - j < (*metadata)[i]->prefsOffset + (*metadata)[i]->prefsCount; j++) { + int32_t offset = (*metadata)[i]->prefsOffset; + int32_t count = (*metadata)[i]->prefsCount; + for (int32_t j = offset; j < offset + count; j++) { auto p = (*unitPrefs)[j]; - logln(" %d: unit %s, geq %f, skeleton \"%s\"", j, p->unit.data(), p->geq, p->skeleton.data()); + logln(" %d: 0x%x unit %s, geq %f, skeleton \"%s\"", j, p, p->unit.data(), p->geq, + p->skeleton.data()); } } for (const auto &t : testCases) { - MaybeStackVector prefs; logln(t.name); - preferences.getPreferencesFor(t.category, t.usage, t.region, &prefs, status); - if (prefs.length() > 0) { + UnitPreference **prefs; + int32_t prefsCount; + preferences.getPreferencesFor(t.category, t.usage, t.region, prefs, prefsCount, status); + if (status.errIfFailureAndReset("getPreferencesFor(\"%s\", \"%s\", \"%s\", ...", t.category, + t.usage, t.region)) { + continue; + } + if (prefsCount > 0) { assertEquals(UnicodeString(t.name) + " - max unit", t.expectedBiggest, prefs[0]->unit.data()); assertEquals(UnicodeString(t.name) + " - min unit", t.expectedSmallest, - prefs[prefs.length() - 1]->unit.data()); + prefs[prefsCount - 1]->unit.data()); } else { errln(UnicodeString(t.name) + ": failed to find preferences"); } -- 2.40.0