From 54d81721cf0d940dbcb6b48dfba5570ca0fc307d Mon Sep 17 00:00:00 2001 From: Quentin Al-Timimi <27322516+quentin987@users.noreply.github.com> Date: Thu, 4 Jun 2020 10:51:06 -0500 Subject: [PATCH] Pre-Unit conversion work refactor, replace category with categoryID (#1260) Removed category as key in category to unit vector map and replaced with category id to reduce memory footprint. --- src/CalcManager/UnitConverter.cpp | 16 +++++++--------- src/CalcManager/UnitConverter.h | 15 +++------------ .../DataLoaders/CurrencyDataLoader.cpp | 4 ++-- .../DataLoaders/CurrencyDataLoader.h | 6 +++--- .../DataLoaders/UnitConverterDataLoader.cpp | 16 ++++++++-------- .../DataLoaders/UnitConverterDataLoader.h | 6 +++--- .../CurrencyConverterUnitTests.cpp | 12 ++++++------ src/CalculatorUnitTests/UnitConverterTest.cpp | 10 +++++----- 8 files changed, 37 insertions(+), 48 deletions(-) diff --git a/src/CalcManager/UnitConverter.cpp b/src/CalcManager/UnitConverter.cpp index ec85e57..d760b61 100644 --- a/src/CalcManager/UnitConverter.cpp +++ b/src/CalcManager/UnitConverter.cpp @@ -109,7 +109,7 @@ CategorySelectionInitializer UnitConverter::SetCurrentCategory(const Category& i { if (m_currentCategory.id != input.id) { - for (auto& unit : m_categoryToUnits[m_currentCategory]) + for (auto& unit : m_categoryToUnits[m_currentCategory.id]) { unit.isConversionSource = (unit.id == m_fromType.id); unit.isConversionTarget = (unit.id == m_toType.id); @@ -121,7 +121,7 @@ CategorySelectionInitializer UnitConverter::SetCurrentCategory(const Category& i } } - newUnitList = m_categoryToUnits[input]; + newUnitList = m_categoryToUnits[input.id]; } InitializeSelectedUnits(); @@ -283,7 +283,7 @@ void UnitConverter::RestoreUserPreferences(wstring_view userPreferences) m_currentCategory = StringToCategory(outerTokens[2]); // Only restore from the saved units if they are valid in the current available units. - auto itr = m_categoryToUnits.find(m_currentCategory); + auto itr = m_categoryToUnits.find(m_currentCategory.id); if (itr != m_categoryToUnits.end()) { const auto& curUnits = itr->second; @@ -713,10 +713,8 @@ vector> UnitConverter::CalculateSuggested() /// void UnitConverter::ResetCategoriesAndRatios() { - m_categories = m_dataLoader->LoadOrderedCategories(); - m_switchedActive = false; - + m_categories = m_dataLoader->GetOrderedCategories(); if (m_categories.empty()) { return; @@ -738,8 +736,8 @@ void UnitConverter::ResetCategoriesAndRatios() continue; } - vector units = activeDataLoader->LoadOrderedUnits(category); - m_categoryToUnits[category] = units; + vector units = activeDataLoader->GetOrderedUnits(category); + m_categoryToUnits[category.id] = units; // Just because the units are empty, doesn't mean the user can't select this category, // we just want to make sure we don't let an unready category be the default. @@ -789,7 +787,7 @@ void UnitConverter::InitializeSelectedUnits() return; } - auto itr = m_categoryToUnits.find(m_currentCategory); + auto itr = m_categoryToUnits.find(m_currentCategory.id); if (itr == m_categoryToUnits.end()) { return; diff --git a/src/CalcManager/UnitConverter.h b/src/CalcManager/UnitConverter.h index d97e9ce..81a2629 100644 --- a/src/CalcManager/UnitConverter.h +++ b/src/CalcManager/UnitConverter.h @@ -116,15 +116,6 @@ namespace UnitConversionManager } }; - class CategoryHash - { - public: - size_t operator()(const Category& x) const - { - return x.id; - } - }; - struct SuggestedValueIntermediate { double magnitude; @@ -171,7 +162,7 @@ namespace UnitConversionManager std::unordered_map, UnitConversionManager::UnitHash> UnitToUnitToConversionDataMap; - typedef std::unordered_map, UnitConversionManager::CategoryHash> + typedef std::unordered_map> CategoryToUnitVectorMap; class IViewModelCurrencyCallback @@ -190,8 +181,8 @@ namespace UnitConversionManager public: virtual ~IConverterDataLoader(){}; virtual void LoadData() = 0; // prepare data if necessary before calling other functions - virtual std::vector LoadOrderedCategories() = 0; - virtual std::vector LoadOrderedUnits(const Category& c) = 0; + virtual std::vector GetOrderedCategories() = 0; + virtual std::vector GetOrderedUnits(const Category& c) = 0; virtual std::unordered_map LoadOrderedRatios(const Unit& u) = 0; virtual bool SupportsCategory(const Category& target) = 0; }; diff --git a/src/CalcViewModel/DataLoaders/CurrencyDataLoader.cpp b/src/CalcViewModel/DataLoaders/CurrencyDataLoader.cpp index d098885..fc91a92 100644 --- a/src/CalcViewModel/DataLoaders/CurrencyDataLoader.cpp +++ b/src/CalcViewModel/DataLoaders/CurrencyDataLoader.cpp @@ -220,14 +220,14 @@ void CurrencyDataLoader::LoadData() }; #pragma optimize("", on) -vector CurrencyDataLoader::LoadOrderedCategories() +vector CurrencyDataLoader::GetOrderedCategories() { // This function should not be called // The model will use the categories from UnitConverterDataLoader return vector(); } -vector CurrencyDataLoader::LoadOrderedUnits(const UCM::Category& /*category*/) +vector CurrencyDataLoader::GetOrderedUnits(const UCM::Category& /*category*/) { lock_guard lock(m_currencyUnitsMutex); return m_currencyUnits; diff --git a/src/CalcViewModel/DataLoaders/CurrencyDataLoader.h b/src/CalcViewModel/DataLoaders/CurrencyDataLoader.h index f4b5082..3216d63 100644 --- a/src/CalcViewModel/DataLoaders/CurrencyDataLoader.h +++ b/src/CalcViewModel/DataLoaders/CurrencyDataLoader.h @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. #pragma once @@ -63,8 +63,8 @@ namespace CalculatorApp // IConverterDataLoader void LoadData() override; - std::vector LoadOrderedCategories() override; - std::vector LoadOrderedUnits(const UCM::Category& category) override; + std::vector GetOrderedCategories() override; + std::vector GetOrderedUnits(const UCM::Category& category) override; std::unordered_map LoadOrderedRatios(const UCM::Unit& unit) override; bool SupportsCategory(const UnitConversionManager::Category& target) override; // IConverterDataLoader diff --git a/src/CalcViewModel/DataLoaders/UnitConverterDataLoader.cpp b/src/CalcViewModel/DataLoaders/UnitConverterDataLoader.cpp index f90260d..145bd27 100644 --- a/src/CalcViewModel/DataLoaders/UnitConverterDataLoader.cpp +++ b/src/CalcViewModel/DataLoaders/UnitConverterDataLoader.cpp @@ -22,18 +22,18 @@ UnitConverterDataLoader::UnitConverterDataLoader(GeographicRegion ^ region) : m_currentRegionCode(region->CodeTwoLetter) { m_categoryList = make_shared>(); - m_categoryToUnits = make_shared(); + m_categoryIDToUnitsMap = make_shared(); m_ratioMap = make_shared(); } -vector UnitConverterDataLoader::LoadOrderedCategories() +vector UnitConverterDataLoader::GetOrderedCategories() { return *m_categoryList; } -vector UnitConverterDataLoader::LoadOrderedUnits(const UCM::Category& category) +vector UnitConverterDataLoader::GetOrderedUnits(const UCM::Category& category) { - return m_categoryToUnits->at(category); + return this->m_categoryIDToUnitsMap->at(category.id); } unordered_map UnitConverterDataLoader::LoadOrderedRatios(const UCM::Unit& unit) @@ -75,8 +75,8 @@ void UnitConverterDataLoader::LoadData() GetConversionData(categoryToUnitConversionDataMap); GetExplicitConversionData(explicitConversionData); // This is needed for temperature conversions - m_categoryToUnits->clear(); - m_ratioMap->clear(); + this->m_categoryIDToUnitsMap->clear(); + this->m_ratioMap->clear(); for (UCM::Category objectCategory : *m_categoryList) { ViewMode categoryViewMode = NavCategory::Deserialize(objectCategory.id); @@ -86,7 +86,7 @@ void UnitConverterDataLoader::LoadData() // Currency is an ordered category but we do not want to process it here // because this function is not thread-safe and currency data is asynchronously // loaded. - m_categoryToUnits->insert(pair>(objectCategory, {})); + this->m_categoryIDToUnitsMap->insert(pair>(objectCategory.id, {})); continue; } @@ -103,7 +103,7 @@ void UnitConverterDataLoader::LoadData() } // Save units per category - m_categoryToUnits->insert(pair>(objectCategory, unitList)); + this->m_categoryIDToUnitsMap->insert(pair>(objectCategory.id, unitList)); // For each unit, populate the conversion data for (UCM::Unit unit : unitList) diff --git a/src/CalcViewModel/DataLoaders/UnitConverterDataLoader.h b/src/CalcViewModel/DataLoaders/UnitConverterDataLoader.h index 58d78e2..0011fdb 100644 --- a/src/CalcViewModel/DataLoaders/UnitConverterDataLoader.h +++ b/src/CalcViewModel/DataLoaders/UnitConverterDataLoader.h @@ -71,8 +71,8 @@ namespace CalculatorApp private: // IConverterDataLoader void LoadData() override; - std::vector LoadOrderedCategories() override; - std::vector LoadOrderedUnits(const UnitConversionManager::Category& c) override; + std::vector GetOrderedCategories() override; + std::vector GetOrderedUnits(const UnitConversionManager::Category& c) override; std::unordered_map LoadOrderedRatios(const UnitConversionManager::Unit& unit) override; bool SupportsCategory(const UnitConversionManager::Category& target) override; @@ -87,7 +87,7 @@ namespace CalculatorApp std::wstring GetLocalizedStringName(_In_ Platform::String ^ stringId); std::shared_ptr> m_categoryList; - std::shared_ptr m_categoryToUnits; + std::shared_ptr m_categoryIDToUnitsMap; std::shared_ptr m_ratioMap; Platform::String ^ m_currentRegionCode; }; diff --git a/src/CalculatorUnitTests/CurrencyConverterUnitTests.cpp b/src/CalculatorUnitTests/CurrencyConverterUnitTests.cpp index a534722..df20b71 100644 --- a/src/CalculatorUnitTests/CurrencyConverterUnitTests.cpp +++ b/src/CalculatorUnitTests/CurrencyConverterUnitTests.cpp @@ -404,7 +404,7 @@ TEST_METHOD(Loaded_LoadOrderedUnits) VERIFY_IS_TRUE(loader.LoadedFromCache()); VERIFY_IS_FALSE(loader.LoadedFromWeb()); - vector unitList = loader.LoadOrderedUnits(CURRENCY_CATEGORY); + vector unitList = loader.GetOrderedUnits(CURRENCY_CATEGORY); VERIFY_ARE_EQUAL(size_t{ 2 }, unitList.size()); const UCM::Unit usdUnit = GetUnit(unitList, L"USD"); @@ -434,7 +434,7 @@ TEST_METHOD(Loaded_LoadOrderedRatios) VERIFY_IS_TRUE(loader.LoadedFromCache()); VERIFY_IS_FALSE(loader.LoadedFromWeb()); - vector unitList = loader.LoadOrderedUnits(CURRENCY_CATEGORY); + vector unitList = loader.GetOrderedUnits(CURRENCY_CATEGORY); VERIFY_ARE_EQUAL(size_t{ 2 }, unitList.size()); const UCM::Unit usdUnit = GetUnit(unitList, L"USD"); @@ -467,7 +467,7 @@ TEST_METHOD(Loaded_GetCurrencySymbols_Valid) VERIFY_IS_TRUE(loader.LoadedFromCache()); VERIFY_IS_FALSE(loader.LoadedFromWeb()); - vector unitList = loader.LoadOrderedUnits(CURRENCY_CATEGORY); + vector unitList = loader.GetOrderedUnits(CURRENCY_CATEGORY); VERIFY_ARE_EQUAL(size_t{ 2 }, unitList.size()); const UCM::Unit usdUnit = GetUnit(unitList, L"USD"); @@ -506,7 +506,7 @@ TEST_METHOD(Loaded_GetCurrencySymbols_Invalid) VERIFY_ARE_EQUAL(wstring(L""), wstring(symbols.second.c_str())); // Verify that when only one unit is valid, both symbols return as empty string. - vector unitList = loader.LoadOrderedUnits(CURRENCY_CATEGORY); + vector unitList = loader.GetOrderedUnits(CURRENCY_CATEGORY); VERIFY_ARE_EQUAL(size_t{ 2 }, unitList.size()); const UCM::Unit usdUnit = GetUnit(unitList, L"USD"); @@ -539,7 +539,7 @@ TEST_METHOD(Loaded_GetCurrencyRatioEquality_Valid) VERIFY_IS_TRUE(loader.LoadedFromCache()); VERIFY_IS_FALSE(loader.LoadedFromWeb()); - vector unitList = loader.LoadOrderedUnits(CURRENCY_CATEGORY); + vector unitList = loader.GetOrderedUnits(CURRENCY_CATEGORY); VERIFY_ARE_EQUAL(size_t{ 2 }, unitList.size()); const UCM::Unit usdUnit = GetUnit(unitList, L"USD"); @@ -577,7 +577,7 @@ TEST_METHOD(Loaded_GetCurrencyRatioEquality_Invalid) VERIFY_ARE_EQUAL(wstring(L""), ratio.second); // Verify that when only one unit is valid, both symbols return as empty string. - vector unitList = loader.LoadOrderedUnits(CURRENCY_CATEGORY); + vector unitList = loader.GetOrderedUnits(CURRENCY_CATEGORY); VERIFY_ARE_EQUAL(size_t{ 2 }, unitList.size()); const UCM::Unit usdUnit = GetUnit(unitList, L"USD"); diff --git a/src/CalculatorUnitTests/UnitConverterTest.cpp b/src/CalculatorUnitTests/UnitConverterTest.cpp index 385f72a..3c4a8f2 100644 --- a/src/CalculatorUnitTests/UnitConverterTest.cpp +++ b/src/CalculatorUnitTests/UnitConverterTest.cpp @@ -60,8 +60,8 @@ namespace UnitConverterUnitTests c2units.push_back(u3); c2units.push_back(u4); - m_units[c1] = c1units; - m_units[c2] = c2units; + m_units[c1.id] = c1units; + m_units[c2.id] = c2units; unordered_map unit1Map = unordered_map(); unordered_map unit2Map = unordered_map(); @@ -99,14 +99,14 @@ namespace UnitConverterUnitTests m_loadDataCallCount++; } - vector LoadOrderedCategories() + vector GetOrderedCategories() { return m_categories; } - vector LoadOrderedUnits(const Category& c) + vector GetOrderedUnits(const Category& category) { - return m_units[c]; + return m_units[category.id]; } unordered_map LoadOrderedRatios(const Unit& u)