Refactoring LocalizationSettings (#850)

src/CalcViewModel/Common/LocalizationSettings.h

@@ -14,45 +14,48 @@ namespace CalculatorApp
         {
         private:
             LocalizationSettings()
-            {
-                int result = 0;
-
                 // Use DecimalFormatter as it respects the locale and the user setting
-                Windows::Globalization::NumberFormatting::DecimalFormatter ^ formatter;
+                : LocalizationSettings(LocalizationService::GetInstance()->GetRegionalSettingsAwareDecimalFormatter())
-                formatter = LocalizationService::GetInstance()->GetRegionalSettingsAwareDecimalFormatter();
+            {
+            }
+
+        public:
+            // This is only public for unit testing purposes.
+            LocalizationSettings(Windows::Globalization::NumberFormatting::DecimalFormatter ^ formatter)
+            {
                 formatter->FractionDigits = 0;
                 formatter->IsDecimalPointAlwaysDisplayed = false;
 
-                for (unsigned int i = 0; i < 10; i++)
+                for (unsigned int i = 0; i < m_digitSymbols.size(); i++)
                 {
-                    m_digitSymbols.at(i) = formatter->FormatUInt(i)->Data()[0];
+                    m_digitSymbols[i] = formatter->FormatUInt(i)->Data()[0];
                 }
 
                 wchar_t resolvedName[LOCALE_NAME_MAX_LENGTH];
-                result = ResolveLocaleName(formatter->ResolvedLanguage->Data(), resolvedName, LOCALE_NAME_MAX_LENGTH);
+                int result = ResolveLocaleName(formatter->ResolvedLanguage->Data(), resolvedName, LOCALE_NAME_MAX_LENGTH);
                 if (result == 0)
                 {
                     throw std::runtime_error("Unexpected error resolving locale name");
                 }
                 else
                 {
-                    m_resolvedName = resolvedName;
+                    m_resolvedName = ref new Platform::String(resolvedName);
                     wchar_t decimalString[LocaleSettingBufferSize] = L"";
-                    result = GetLocaleInfoEx(m_resolvedName.c_str(), LOCALE_SDECIMAL, decimalString, static_cast<int>(std::size(decimalString)));
+                    result = GetLocaleInfoEx(m_resolvedName->Data(), LOCALE_SDECIMAL, decimalString, static_cast<int>(std::size(decimalString)));
                     if (result == 0)
                     {
                         throw std::runtime_error("Unexpected error while getting locale info");
                     }
 
                     wchar_t groupingSymbolString[LocaleSettingBufferSize] = L"";
-                    result = GetLocaleInfoEx(m_resolvedName.c_str(), LOCALE_STHOUSAND, groupingSymbolString, static_cast<int>(std::size(groupingSymbolString)));
+                    result = GetLocaleInfoEx(m_resolvedName->Data(), LOCALE_STHOUSAND, groupingSymbolString, static_cast<int>(std::size(groupingSymbolString)));
                     if (result == 0)
                     {
                         throw std::runtime_error("Unexpected error while getting locale info");
                     }
 
                     wchar_t numberGroupingString[LocaleSettingBufferSize] = L"";
-                    result = GetLocaleInfoEx(m_resolvedName.c_str(), LOCALE_SGROUPING, numberGroupingString, static_cast<int>(std::size(numberGroupingString)));
+                    result = GetLocaleInfoEx(m_resolvedName->Data(), LOCALE_SGROUPING, numberGroupingString, static_cast<int>(std::size(numberGroupingString)));
                     if (result == 0)
                     {
                         throw std::runtime_error("Unexpected error while getting locale info");
@@ -61,7 +64,7 @@ namespace CalculatorApp
                     // Get locale info for List Separator, eg. comma is used in many locales
                     wchar_t listSeparatorString[4] = L"";
                     result = ::GetLocaleInfoEx(
-                        m_resolvedName.c_str(),
+                        m_resolvedName->Data(),
                         LOCALE_SLIST,
                         listSeparatorString,
                         static_cast<int>(std::size(listSeparatorString))); // Max length of the expected return value is 4
@@ -72,7 +75,7 @@ namespace CalculatorApp
 
                     int currencyTrailingDigits = 0;
                     result = GetLocaleInfoEx(
-                        m_resolvedName.c_str(),
+                        m_resolvedName->Data(),
                         LOCALE_ICURRDIGITS | LOCALE_RETURN_NUMBER,
                         (LPWSTR)&currencyTrailingDigits,
                         sizeof(currencyTrailingDigits) / sizeof(WCHAR));
@@ -85,7 +88,7 @@ namespace CalculatorApp
                     // A value of 0 indicates the symbol follows the currency value.
                     int currencySymbolPrecedence = 1;
                     result = GetLocaleInfoEx(
-                        m_resolvedName.c_str(),
+                        m_resolvedName->Data(),
                         LOCALE_IPOSSYMPRECEDES | LOCALE_RETURN_NUMBER,
                         (LPWSTR)&currencySymbolPrecedence,
                         sizeof(currencySymbolPrecedence) / sizeof(WCHAR));
@@ -104,14 +107,14 @@ namespace CalculatorApp
                 // Note: This function returns 0 on failure.
                 // We'll ignore the failure in that case and the CalendarIdentifier would get set to GregorianCalendar.
                 CALID calId;
-                ::GetLocaleInfoEx(m_resolvedName.c_str(), LOCALE_ICALENDARTYPE | LOCALE_RETURN_NUMBER, reinterpret_cast<PWSTR>(&calId), sizeof(calId));
+                ::GetLocaleInfoEx(m_resolvedName->Data(), LOCALE_ICALENDARTYPE | LOCALE_RETURN_NUMBER, reinterpret_cast<PWSTR>(&calId), sizeof(calId));
 
                 m_calendarIdentifier = GetCalendarIdentifierFromCalid(calId);
 
                 // Get FirstDayOfWeek Date and Time setting
                 wchar_t day[80] = L"";
                 ::GetLocaleInfoEx(
-                    m_resolvedName.c_str(),
+                    m_resolvedName->Data(),
                     LOCALE_IFIRSTDAYOFWEEK,            // The first day in a week
                     reinterpret_cast<PWSTR>(day),      // Argument is of type PWSTR
                     static_cast<int>(std::size(day))); // Max return size are 80 characters
@@ -122,7 +125,6 @@ namespace CalculatorApp
                 m_firstDayOfWeek = static_cast<Windows::Globalization::DayOfWeek>((_wtoi(day) + 1) % 7); // static cast int to DayOfWeek enum
             }
 
-        public:
             // A LocalizationSettings object is not copyable.
             LocalizationSettings(const LocalizationSettings&) = delete;
             LocalizationSettings& operator=(const LocalizationSettings&) = delete;
@@ -141,16 +143,12 @@ namespace CalculatorApp
 
             Platform::String ^ GetLocaleName() const
             {
-                return ref new Platform::String(m_resolvedName.c_str());
+                return m_resolvedName;
             }
 
             bool IsDigitEnUsSetting() const
             {
-                if (this->GetDigitSymbolFromEnUsDigit('0') == L'0')
+                return (this->GetDigitSymbolFromEnUsDigit('0') == L'0');
-                {
-                    return true;
-                }
-                return false;
             }
 
             void LocalizeDisplayValue(_Inout_ std::wstring* stringToLocalize) const
@@ -176,56 +174,39 @@ namespace CalculatorApp
                     return localizedString;
                 }
 
-                size_t i = 0;
+                std::wstring englishString;
-                auto localizedStringData = localizedString->Data();
+                englishString.reserve(localizedString->Length());
-                size_t length = localizedString->Length();
-                std::unique_ptr<wchar_t[]> englishString(new wchar_t[length + 1]); // +1 for the null termination
 
-                for (; i < length; ++i)
+                for (wchar_t ch : localizedString)
                 {
-                    wchar_t ch = localizedStringData[i];
                     if (!IsEnUsDigit(ch))
                     {
-                        for (int j = 0; j < 10; ++j)
+                        auto it = std::find(m_digitSymbols.begin(), m_digitSymbols.end(), ch);
+
+                        if (it != m_digitSymbols.end())
                         {
-                            if (ch == m_digitSymbols[j])
+                            auto index = std::distance(m_digitSymbols.begin(), it);
-                            {
+                            ch = index.ToString()->Data()[0];
-                                ch = j.ToString()->Data()[0];
-                                break;
-                                // ch = val - L'0';
-                            }
                         }
                     }
                     if (ch == m_decimalSeparator)
                     {
                         ch = L'.';
                     }
-                    englishString[i] = ch;
+                    englishString += ch;
                 }
-                englishString[i] = '\0';
 
-                return ref new Platform::String(englishString.get());
+                return ref new Platform::String(englishString.c_str());
             }
 
             bool IsEnUsDigit(const wchar_t digit) const
             {
-                if (digit >= L'0' && digit <= L'9')
+                return (digit >= L'0' && digit <= L'9');
-                {
-                    return true;
-                }
-                return false;
             }
 
             bool IsLocalizedDigit(const wchar_t digit) const
             {
-                for (auto dig : m_digitSymbols)
+                return std::find(m_digitSymbols.begin(), m_digitSymbols.end(), digit) != m_digitSymbols.end();
-                {
-                    if (digit == dig)
-                    {
-                        return true;
-                    }
-                }
-                return false;
             }
 
             bool IsLocalizedHexDigit(const wchar_t digit) const
@@ -235,15 +216,7 @@ namespace CalculatorApp
                     return true;
                 }
 
-                for (auto dig : s_hexSymbols)
+                return std::find(s_hexSymbols.begin(), s_hexSymbols.end(), digit) != s_hexSymbols.end();
-                {
-                    if (digit == dig)
-                    {
-                        return true;
-                    }
-                }
-
-                return false;
             }
 
             wchar_t GetDigitSymbolFromEnUsDigit(wchar_t digitSymbol) const
@@ -265,16 +238,12 @@ namespace CalculatorApp
 
             std::wstring GetDecimalSeparatorStr() const
             {
-                std::wstring result;
+                return std::wstring(1, m_decimalSeparator);
-                result.push_back(m_decimalSeparator);
-                return result;
             }
 
             std::wstring GetNumberGroupingSeparatorStr() const
             {
-                std::wstring result;
+                return std::wstring(1, m_numberGroupSeparator);
-                result.push_back(m_numberGroupSeparator);
-                return result;
             }
 
             std::wstring GetNumberGroupingStr() const
@@ -284,15 +253,11 @@ namespace CalculatorApp
 
             Platform::String ^ RemoveGroupSeparators(Platform::String ^ source) const
             {
-                std::wstringstream stream;
+                std::wstring destination;
-                for (auto c = source->Begin(); c < source->End(); ++c)
+                std::copy_if(
-                {
+                    begin(source), end(source), std::back_inserter(destination), [this](auto const c) { return c != L' ' && c != m_numberGroupSeparator; });
-                    if (*c != L' ' && *c != m_numberGroupSeparator)
+                
-                    {
+                return ref new Platform::String(destination.c_str());
-                        stream << *c;
-                    }
-                }
-                return ref new Platform::String(stream.str().c_str());
             }
 
             Platform::String ^ GetCalendarIdentifier() const
@@ -371,7 +336,7 @@ namespace CalculatorApp
             Platform::String ^ m_calendarIdentifier;
             Windows::Globalization::DayOfWeek m_firstDayOfWeek;
             int m_currencySymbolPrecedence;
-            std::wstring m_resolvedName;
+            Platform::String ^ m_resolvedName;
             int m_currencyTrailingDigits;
             static const unsigned int LocaleSettingBufferSize = 16;
         };

src/CalculatorUnitTests/CalculatorUnitTests.vcxproj

@@ -244,6 +244,7 @@
     <ClCompile Include="DateCalculatorUnitTests.cpp" />
     <ClCompile Include="HistoryTests.cpp" />
     <ClCompile Include="LocalizationServiceUnitTests.cpp" />
+    <ClCompile Include="LocalizationSettingsUnitTests.cpp" />
     <ClCompile Include="Mocks\CurrencyHttpClient.cpp" />
     <ClCompile Include="MultiWindowUnitTests.cpp" />
     <ClCompile Include="NavCategoryUnitTests.cpp" />

src/CalculatorUnitTests/CalculatorUnitTests.vcxproj.filters

@@ -29,6 +29,7 @@
     </ClCompile>
     <ClCompile Include="LocalizationServiceUnitTests.cpp" />
     <ClCompile Include="RationalTest.cpp" />
+    <ClCompile Include="LocalizationSettingsUnitTests.cpp" />
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="DateUtils.h" />

src/CalculatorUnitTests/LocalizationSettingsUnitTests.cpp

@@ -0,0 +1,93 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+#include "pch.h"
+
+#include <CppUnitTest.h>
+
+#include "CalcViewModel/Common/LocalizationSettings.h"
+
+using namespace CalculatorApp::Common;
+using namespace Microsoft::VisualStudio::CppUnitTestFramework;
+
+static Windows::Globalization::NumberFormatting::DecimalFormatter ^ CreateDecimalFormatter(const wchar_t* language, const wchar_t* geographicRegion)
+{
+    auto languages = ref new Platform::Collections::Vector<Platform::String ^>({ ref new Platform::String(language) });
+    return ref new Windows::Globalization::NumberFormatting::DecimalFormatter(languages, ref new Platform::String(geographicRegion));
+}
+
+namespace CalculatorUnitTests
+{
+    TEST_CLASS(LocalizationSettingsUnitTests)
+    {
+    public:
+        TEST_METHOD(TestLocaleName)
+        {
+            auto formatter = CreateDecimalFormatter(L"en-US", L"US");
+            LocalizationSettings settings(formatter);
+            VERIFY_ARE_EQUAL(L"en-US", settings.GetLocaleName());
+        }
+
+        TEST_METHOD(TestIsDigitEnUsSetting)
+        {
+            auto arabicFormatter = CreateDecimalFormatter(L"ar-AE", L"AE");
+            LocalizationSettings arabicSettings(arabicFormatter);
+            VERIFY_IS_FALSE(arabicSettings.IsDigitEnUsSetting());
+
+            auto englishFormatter = CreateDecimalFormatter(L"en-US", L"US");
+            LocalizationSettings englishSettings(englishFormatter);
+            VERIFY_IS_TRUE(englishSettings.IsDigitEnUsSetting());
+        }
+
+        TEST_METHOD(TestLocalizeDisplayValue)
+        {
+            auto formatter = CreateDecimalFormatter(L"ar-AE", L"AE");
+            LocalizationSettings settings(formatter);
+            std::wstring input(L"A123");
+            settings.LocalizeDisplayValue(&input);
+            VERIFY_ARE_EQUAL(L"A١٢٣", input);
+        }
+
+        TEST_METHOD(TestGetEnglishValueFromLocalizedDigits)
+        {
+            auto formatter = CreateDecimalFormatter(L"ar-AE", L"AE");
+            LocalizationSettings settings(formatter);
+            VERIFY_ARE_EQUAL(L"A123", settings.GetEnglishValueFromLocalizedDigits(L"A١٢٣"));
+        }
+
+        TEST_METHOD(TestIsEnUsDigit)
+        {
+            auto& settings = LocalizationSettings::GetInstance();
+            VERIFY_IS_FALSE(settings.IsEnUsDigit(L'/'));
+            VERIFY_IS_TRUE(settings.IsEnUsDigit(L'0'));
+            VERIFY_IS_TRUE(settings.IsEnUsDigit(L'1'));
+            VERIFY_IS_TRUE(settings.IsEnUsDigit(L'8'));
+            VERIFY_IS_TRUE(settings.IsEnUsDigit(L'9'));
+            VERIFY_IS_FALSE(settings.IsEnUsDigit(L':'));
+        }
+
+        TEST_METHOD(TestIsLocalizedDigit)
+        {
+            auto formatter = CreateDecimalFormatter(L"en-US", L"US");
+            LocalizationSettings settings(formatter);
+            VERIFY_IS_TRUE(settings.IsLocalizedDigit(L'0'));
+            VERIFY_IS_FALSE(settings.IsLocalizedDigit(L'A'));
+        }
+
+        TEST_METHOD(TestIsLocalizedHexDigit)
+        {
+            auto formatter = CreateDecimalFormatter(L"en-US", L"US");
+            LocalizationSettings settings(formatter);
+            VERIFY_IS_TRUE(settings.IsLocalizedHexDigit(L'0'));
+            VERIFY_IS_TRUE(settings.IsLocalizedHexDigit(L'A'));
+            VERIFY_IS_FALSE(settings.IsLocalizedHexDigit(L'G'));
+        }
+
+        TEST_METHOD(TestRemoveGroupSeparators)
+        {
+            auto formatter = CreateDecimalFormatter(L"en-US", L"US");
+            LocalizationSettings settings(formatter);
+            VERIFY_ARE_EQUAL(L"1000000", settings.RemoveGroupSeparators(L"1,000 000"));
+        }
+    };
+}