Unit Converter - Calculate the rounding precision of results based on the source (#498)

* precisely calculates the number of digits of the source and compute the number of significant digits of the result based on that.

* fix unit test

* Fix warning C4267

* Optimize how we calculate the number of digits in the integer part and don't trim the value when used by the currency converter

* modify GetNumberSignificantDigits

* fix CI error

* Access to wstring::npos from static calls

* Move UnitConverter static methods related to number to NumberFormattingUtils

* rename namespace

* Add comment and fix typo

* Move standard headers

src/CalcManager/CalcManager.vcxproj

@@ -309,6 +309,7 @@
     <ClInclude Include="Ratpack\CalcErr.h" />
     <ClInclude Include="Ratpack\ratconst.h" />
     <ClInclude Include="Ratpack\ratpak.h" />
+    <ClInclude Include="NumberFormattingUtils.h" />
     <ClInclude Include="UnitConverter.h" />
   </ItemGroup>
   <ItemGroup>
@@ -349,6 +350,7 @@
       <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">Create</PrecompiledHeader>
       <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Release|x64'">Create</PrecompiledHeader>
     </ClCompile>
+    <ClCompile Include="NumberFormattingUtils.cpp" />
     <ClCompile Include="UnitConverter.cpp" />
   </ItemGroup>
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />

src/CalcManager/CalcManager.vcxproj.filters

@@ -89,6 +89,7 @@
     <ClCompile Include="CEngine\RationalMath.cpp">
       <Filter>CEngine</Filter>
     </ClCompile>
+    <ClCompile Include="NumberFormattingUtils.cpp" />
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="Command.h" />
@@ -160,5 +161,6 @@
     <ClInclude Include="Header Files\RationalMath.h">
       <Filter>Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="NumberFormattingUtils.h" />
   </ItemGroup>
-</Project>
+</Project>

src/CalcManager/NumberFormattingUtils.cpp

@@ -0,0 +1,84 @@
+#include "pch.h"
+#include "NumberFormattingUtils.h"
+
+using namespace std;
+
+namespace CalcManager::NumberFormattingUtils
+{
+    /// <summary>
+    /// Trims out any trailing zeros or decimals in the given input string
+    /// </summary>
+    /// <param name="number">number to trim</param>
+    void TrimTrailingZeros(_Inout_ wstring& number)
+    {
+        if (number.find(L'.') == wstring::npos)
+        {
+            return;
+        }
+
+        wstring::iterator iter;
+        for (iter = number.end() - 1;; iter--)
+        {
+            if (*iter != L'0')
+            {
+                number.erase(iter + 1, number.end());
+                break;
+            }
+        }
+        if (*(number.end() - 1) == L'.')
+        {
+            number.erase(number.end() - 1, number.end());
+        }
+    }
+
+    /// <summary>
+    /// Get number of digits (whole number part + decimal part)</summary>
+    /// <param name="value">the number</param>
+    unsigned int GetNumberDigits(wstring value)
+    {
+        TrimTrailingZeros(value);
+        unsigned int numberSignificantDigits = static_cast<unsigned int>(value.size());
+        if (value.find(L'.') != wstring::npos)
+        {
+            --numberSignificantDigits;
+        }
+        if (value.find(L'-') != wstring::npos)
+        {
+            --numberSignificantDigits;
+        }
+        return numberSignificantDigits;
+    }
+
+    /// <summary>
+    /// Get number of digits (whole number part only)</summary>
+    /// <param name="value">the number</param>
+    unsigned int GetNumberDigitsWholeNumberPart(double value)
+    {
+        return value == 0 ? 1 : (1 + (int)log10(abs(value)));
+    }
+
+    /// <summary>
+    /// Rounds the given double to the given number of significant digits
+    /// </summary>
+    /// <param name="num">input double</param>
+    /// <param name="numSignificant">int number of significant digits to round to</param>
+    wstring RoundSignificantDigits(double num, int numSignificant)
+    {
+        wstringstream out(wstringstream::out);
+        out << fixed;
+        out.precision(numSignificant);
+        out << num;
+        return out.str();
+    }
+
+    /// <summary>
+    ///  Convert a Number to Scientific Notation
+    /// </summary>
+    /// <param name="number">number to convert</param>
+    wstring ToScientificNumber(double number)
+    {
+        wstringstream out(wstringstream::out);
+        out << scientific << number;
+        return out.str();
+    }
+}

src/CalcManager/NumberFormattingUtils.h

@@ -0,0 +1,15 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+#pragma once
+
+#include <string>
+
+namespace CalcManager::NumberFormattingUtils
+{
+    void TrimTrailingZeros(_Inout_ std::wstring& input);
+    unsigned int GetNumberDigits(std::wstring value);
+    unsigned int GetNumberDigitsWholeNumberPart(double value);
+    std::wstring RoundSignificantDigits(double value, int numberSignificantDigits);
+    std::wstring ToScientificNumber(double number);
+}

src/CalcManager/UnitConverter.cpp

@@ -7,9 +7,11 @@
 #include <algorithm> // for std::sort
 #include "Command.h"
 #include "UnitConverter.h"
+#include "NumberFormattingUtils.h"
 
 using namespace std;
 using namespace UnitConversionManager;
+using namespace CalcManager::NumberFormattingUtils;
 
 static constexpr uint32_t EXPECTEDSERIALIZEDCATEGORYTOKENCOUNT = 3;
 static constexpr uint32_t EXPECTEDSERIALIZEDUNITTOKENCOUNT = 6;
@@ -178,7 +180,7 @@ void UnitConverter::SwitchActive(const wstring& newValue)
     swap(m_currentHasDecimal, m_returnHasDecimal);
     m_returnDisplay = m_currentDisplay;
     m_currentDisplay = newValue;
-    m_currentHasDecimal = (m_currentDisplay.find(L'.') != m_currentDisplay.npos);
+    m_currentHasDecimal = (m_currentDisplay.find(L'.') != wstring::npos);
     m_switchedActive = true;
 
     if (m_currencyDataLoader != nullptr && m_vmCurrencyCallback != nullptr)
@@ -202,7 +204,7 @@ vector<wstring> UnitConverter::StringToVector(const wstring& w, const wchar_t* d
     size_t delimiterIndex = w.find(delimiter);
     size_t startIndex = 0;
     vector<wstring> serializedTokens = vector<wstring>();
-    while (delimiterIndex != w.npos)
+    while (delimiterIndex != wstring::npos)
     {
         serializedTokens.push_back(w.substr(startIndex, delimiterIndex - startIndex));
         startIndex = delimiterIndex + (int)wcslen(delimiter);
@@ -634,19 +636,19 @@ vector<tuple<wstring, Unit>> UnitConverter::CalculateSuggested()
         wstring roundedString;
         if (abs(entry.value) < 100)
         {
-            roundedString = RoundSignificant(entry.value, 2);
+            roundedString = RoundSignificantDigits(entry.value, 2);
         }
         else if (abs(entry.value) < 1000)
         {
-            roundedString = RoundSignificant(entry.value, 1);
+            roundedString = RoundSignificantDigits(entry.value, 1);
         }
         else
         {
-            roundedString = RoundSignificant(entry.value, 0);
+            roundedString = RoundSignificantDigits(entry.value, 0);
         }
         if (stod(roundedString) != 0.0 || m_currentCategory.supportsNegative)
         {
-            TrimString(roundedString);
+            TrimTrailingZeros(roundedString);
             returnVector.push_back(make_tuple(roundedString, entry.type));
         }
     }
@@ -672,21 +674,21 @@ vector<tuple<wstring, Unit>> UnitConverter::CalculateSuggested()
         wstring roundedString;
         if (abs(entry.value) < 100)
         {
-            roundedString = RoundSignificant(entry.value, 2);
+            roundedString = RoundSignificantDigits(entry.value, 2);
         }
         else if (abs(entry.value) < 1000)
         {
-            roundedString = RoundSignificant(entry.value, 1);
+            roundedString = RoundSignificantDigits(entry.value, 1);
         }
         else
         {
-            roundedString = RoundSignificant(entry.value, 0);
+            roundedString = RoundSignificantDigits(entry.value, 0);
         }
 
         // How to work out which is the best whimsical value to add to the vector?
         if (stod(roundedString) != 0.0)
         {
-            TrimString(roundedString);
+            TrimTrailingZeros(roundedString);
             whimsicalReturnVector.push_back(make_tuple(roundedString, entry.type));
         }
     }
@@ -843,100 +845,61 @@ void UnitConverter::Calculate()
     {
         m_returnDisplay = m_currentDisplay;
         m_returnHasDecimal = m_currentHasDecimal;
-        TrimString(m_returnDisplay);
+        TrimTrailingZeros(m_returnDisplay);
         UpdateViewModel();
         return;
     }
 
     unordered_map<Unit, ConversionData, UnitHash> conversionTable = m_ratioMap[m_fromType];
-    double returnValue = stod(m_currentDisplay);
+    if (AnyUnitIsEmpty() || (conversionTable[m_toType].ratio == 1.0 && conversionTable[m_toType].offset == 0.0))
-    if (conversionTable[m_toType].ratio == 1.0 && conversionTable[m_toType].offset == 0.0)
     {
         m_returnDisplay = m_currentDisplay;
         m_returnHasDecimal = m_currentHasDecimal;
-        TrimString(m_returnDisplay);
+        TrimTrailingZeros(m_returnDisplay);
     }
     else
     {
-        returnValue = Convert(returnValue, conversionTable[m_toType]);
+        double currentValue = stod(m_currentDisplay);
-        m_returnDisplay = RoundSignificant(returnValue, MAXIMUMDIGITSALLOWED);
+        double returnValue = Convert(currentValue, conversionTable[m_toType]);
-        TrimString(m_returnDisplay);
-        int numPreDecimal = (int)m_returnDisplay.size();
-        if (m_returnDisplay.find(L'.') != m_returnDisplay.npos)
-        {
-            numPreDecimal = (int)m_returnDisplay.find(L'.');
-        }
-        if (returnValue < 0)
-        {
-            numPreDecimal--;
-        }
 
-        if (numPreDecimal > MAXIMUMDIGITSALLOWED || (returnValue != 0 && abs(returnValue) < MINIMUMDECIMALALLOWED))
+        auto isCurrencyConverter = m_currencyDataLoader != nullptr && m_currencyDataLoader->SupportsCategory(this->m_currentCategory);
+        if (isCurrencyConverter)
         {
-            wstringstream out(wstringstream::out);
+            // We don't need to trim the value when it's a currency.
-            out << scientific << returnValue;
+            m_returnDisplay = RoundSignificantDigits(returnValue, MAXIMUMDIGITSALLOWED);
-            m_returnDisplay = out.str();
+            TrimTrailingZeros(m_returnDisplay);
         }
         else
         {
-            returnValue = stod(m_returnDisplay);
+            int numPreDecimal = GetNumberDigitsWholeNumberPart(returnValue);
-            wstring returnString;
+            if (numPreDecimal > MAXIMUMDIGITSALLOWED || (returnValue != 0 && abs(returnValue) < MINIMUMDECIMALALLOWED))
-            if (m_currentDisplay.size() <= OPTIMALDIGITSALLOWED && abs(returnValue) >= OPTIMALDECIMALALLOWED)
             {
-                returnString = RoundSignificant(returnValue, OPTIMALDIGITSALLOWED - min(numPreDecimal, OPTIMALDIGITSALLOWED));
+                m_returnDisplay = ToScientificNumber(returnValue);
             }
             else
             {
-                returnString = RoundSignificant(returnValue, MAXIMUMDIGITSALLOWED - min(numPreDecimal, MAXIMUMDIGITSALLOWED));
+                int currentNumberSignificantDigits = GetNumberDigits(m_currentDisplay);
+                int precision = 0;
+                if (abs(returnValue) < OPTIMALDECIMALALLOWED)
+                {
+                    precision = MAXIMUMDIGITSALLOWED;
+                }
+                else
+                {
+					// Fewer digits are needed following the decimal if the number is large,
+					// we calculate the number of decimals necessary based on the number of digits in the integer part.
+                    precision = max(0, max(OPTIMALDIGITSALLOWED, min(MAXIMUMDIGITSALLOWED, currentNumberSignificantDigits)) - numPreDecimal);
+                }
+
+                m_returnDisplay = RoundSignificantDigits(returnValue, precision);
+                TrimTrailingZeros(m_returnDisplay);
             }
-            m_returnDisplay = returnString;
+            m_returnHasDecimal = (m_returnDisplay.find(L'.') != wstring::npos);
-            TrimString(m_returnDisplay);
         }
-        m_returnHasDecimal = (m_returnDisplay.find(L'.') != m_returnDisplay.npos);
     }
     UpdateViewModel();
 }
 
-/// <summary>
-/// Trims out any trailing zeros or decimals in the given input string
-/// </summary>
-/// <param name="input">wstring to trim</param>
-void UnitConverter::TrimString(wstring& returnString)
-{
-    if (returnString.find(L'.') == m_returnDisplay.npos)
-    {
-        return;
-    }
-
-    wstring::iterator iter;
-    for (iter = returnString.end() - 1;; iter--)
-    {
-        if (*iter != L'0')
-        {
-            returnString.erase(iter + 1, returnString.end());
-            break;
-        }
-    }
-    if (*(returnString.end() - 1) == L'.')
-    {
-        returnString.erase(returnString.end() - 1, returnString.end());
-    }
-}
-
-/// <summary>
-/// Rounds the given double to the given number of significant digits
-/// </summary>
-/// <param name="num">input double</param>
-/// <param name="numSignificant">int number of significant digits to round to</param>
-wstring UnitConverter::RoundSignificant(double num, int numSignificant)
-{
-    wstringstream out(wstringstream::out);
-    out << fixed;
-    out.precision(numSignificant);
-    out << num;
-    return out.str();
-}
-
 void UnitConverter::UpdateCurrencySymbols()
 {
     if (m_currencyDataLoader != nullptr && m_vmCurrencyCallback != nullptr)

src/CalcManager/UnitConverter.h

@@ -279,9 +279,7 @@ namespace UnitConversionManager
         double Convert(double value, ConversionData conversionData);
         std::vector<std::tuple<std::wstring, Unit>> CalculateSuggested();
         void ClearValues();
-        void TrimString(std::wstring& input);
         void InitializeSelectedUnits();
-        std::wstring RoundSignificant(double num, int numSignificant);
         Category StringToCategory(const std::wstring& w);
         std::wstring CategoryToString(const Category& c, const wchar_t* delimiter);
         std::wstring UnitToString(const Unit& u, const wchar_t* delimiter);

src/CalcManager/pch.h

@@ -20,3 +20,5 @@
 #include <unordered_map>
 #include <vector>
 #include <winerror.h>
+#include <iostream>
+#include <math.h>

src/CalculatorUnitTests/CalculatorManagerTest.cpp

@@ -7,9 +7,11 @@
 
 #include "CalcManager/CalculatorHistory.h"
 #include "CalcViewModel/Common/EngineResourceProvider.h"
+#include "CalcManager/NumberFormattingUtils.h"
 
 using namespace CalculatorApp;
 using namespace CalculationManager;
+using namespace CalcManager::NumberFormattingUtils;
 using namespace Platform;
 using namespace std;
 using namespace Microsoft::VisualStudio::CppUnitTestFramework;
@@ -185,6 +187,11 @@ namespace CalculatorManagerTest
         TEST_METHOD(CalculatorManagerTestMaxDigitsReached_LeadingDecimal);
         TEST_METHOD(CalculatorManagerTestMaxDigitsReached_TrailingDecimal);
 
+        TEST_METHOD(CalculatorManagerNumberFormattingUtils_TrimTrailingZeros);
+        TEST_METHOD(CalculatorManagerNumberFormattingUtils_GetNumberDigits);
+        TEST_METHOD(CalculatorManagerNumberFormattingUtils_GetNumberDigitsWholeNumberPart);
+        TEST_METHOD(CalculatorManagerNumberFormattingUtils_RoundSignificantDigits);
+        TEST_METHOD(CalculatorManagerNumberFormattingUtils_ToScientificNumber);
         // TODO re-enable when cause of failure is determined. Bug 20226670
         // TEST_METHOD(CalculatorManagerTestBinaryOperatorReceived);
         // TEST_METHOD(CalculatorManagerTestBinaryOperatorReceived_Multiple);
@@ -807,6 +814,102 @@ namespace CalculatorManagerTest
         TestMaxDigitsReachedScenario(L"123,456,789,101,112.13");
     }
 
+    void CalculatorManagerTest::CalculatorManagerNumberFormattingUtils_TrimTrailingZeros()
+    {
+        wstring number = L"2.1032100000000";
+        TrimTrailingZeros(number);
+        VERIFY_ARE_EQUAL(number, L"2.10321");
+        number = L"-122.123200";
+        TrimTrailingZeros(number);
+        VERIFY_ARE_EQUAL(number, L"-122.1232");
+        number = L"0.0001200";
+        TrimTrailingZeros(number);
+        VERIFY_ARE_EQUAL(number, L"0.00012");
+        number = L"12.000";
+        TrimTrailingZeros(number);
+        VERIFY_ARE_EQUAL(number, L"12");
+        number = L"-12.00000";
+        TrimTrailingZeros(number);
+        VERIFY_ARE_EQUAL(number, L"-12");
+        number = L"0.000";
+        TrimTrailingZeros(number);
+        VERIFY_ARE_EQUAL(number, L"0");
+        number = L"322423";
+        TrimTrailingZeros(number);
+        VERIFY_ARE_EQUAL(number, L"322423");
+    }
+
+    void CalculatorManagerTest::CalculatorManagerNumberFormattingUtils_GetNumberDigits()
+    {
+        wstring number = L"2.10321";
+        unsigned int digitsCount = GetNumberDigits(number);
+        VERIFY_ARE_EQUAL(digitsCount, 6);
+        number = L"-122.1232";
+        digitsCount = GetNumberDigits(number);
+        VERIFY_ARE_EQUAL(digitsCount, 7);
+        number = L"-3432";
+        digitsCount = GetNumberDigits(number);
+        VERIFY_ARE_EQUAL(digitsCount, 4);
+        number = L"0";
+        digitsCount = GetNumberDigits(number);
+        VERIFY_ARE_EQUAL(digitsCount, 1);
+        number = L"0.0001223";
+        digitsCount = GetNumberDigits(number);
+        VERIFY_ARE_EQUAL(digitsCount, 8);
+    }
+
+    void CalculatorManagerTest::CalculatorManagerNumberFormattingUtils_GetNumberDigitsWholeNumberPart()
+    {
+        unsigned int digitsCount = GetNumberDigitsWholeNumberPart(2.10321);
+        VERIFY_ARE_EQUAL(digitsCount, 1);
+        digitsCount = GetNumberDigitsWholeNumberPart(-122.1232);
+        VERIFY_ARE_EQUAL(digitsCount, 3);
+        digitsCount = GetNumberDigitsWholeNumberPart(-3432);
+        VERIFY_ARE_EQUAL(digitsCount, 4);
+        digitsCount = GetNumberDigitsWholeNumberPart(0);
+        VERIFY_ARE_EQUAL(digitsCount, 1);
+        digitsCount = GetNumberDigitsWholeNumberPart(324328412837382);
+        VERIFY_ARE_EQUAL(digitsCount, 15);
+        digitsCount = GetNumberDigitsWholeNumberPart(324328412837382.232213214324234);
+        VERIFY_ARE_EQUAL(digitsCount, 15);
+    }
+
+    void CalculatorManagerTest::CalculatorManagerNumberFormattingUtils_RoundSignificantDigits()
+    {
+        wstring result = RoundSignificantDigits(12.342343242, 3);
+        VERIFY_ARE_EQUAL(result, L"12.342");
+        result = RoundSignificantDigits(12.3429999, 3);
+        VERIFY_ARE_EQUAL(result, L"12.343");
+        result = RoundSignificantDigits(12.342500001, 3);
+        VERIFY_ARE_EQUAL(result, L"12.343");
+        result = RoundSignificantDigits(-2312.1244243346454345, 5);
+        VERIFY_ARE_EQUAL(result, L"-2312.12442");
+        result = RoundSignificantDigits(0.3423432423, 5);
+        VERIFY_ARE_EQUAL(result, L"0.34234");
+        result = RoundSignificantDigits(0.3423, 7);
+        VERIFY_ARE_EQUAL(result, L"0.3423000");
+    }
+
+    void CalculatorManagerTest::CalculatorManagerNumberFormattingUtils_ToScientificNumber()
+    {
+        wstring result = ToScientificNumber(3423);
+        VERIFY_ARE_EQUAL(result, L"3.423000e+03");
+        result = ToScientificNumber(-21);
+        VERIFY_ARE_EQUAL(result, L"-2.100000e+01");
+        result = ToScientificNumber(0.0232);
+        VERIFY_ARE_EQUAL(result, L"2.320000e-02");
+        result = ToScientificNumber(-0.00921);
+        VERIFY_ARE_EQUAL(result, L"-9.210000e-03");
+        result = ToScientificNumber(2343243345677);
+        VERIFY_ARE_EQUAL(result, L"2.343243e+12");
+        result = ToScientificNumber(-3432474247332942);
+        VERIFY_ARE_EQUAL(result, L"-3.432474e+15");
+        result = ToScientificNumber(0.000000003432432);
+        VERIFY_ARE_EQUAL(result, L"3.432432e-09");
+        result = ToScientificNumber(-0.000000003432432);
+        VERIFY_ARE_EQUAL(result, L"-3.432432e-09");
+    }
+
     // TODO re-enable when cause of failure is determined. Bug 20226670
     // void CalculatorManagerTest::CalculatorManagerTestBinaryOperatorReceived()
     // {

src/CalculatorUnitTests/UnitConverterTest.cpp

@@ -441,7 +441,7 @@ namespace UnitConverterUnitTests
         s_unitConverter->SendCommand(Command::Six);
         s_unitConverter->SendCommand(Command::Seven);
         s_unitConverter->SendCommand(Command::Eight);
-        VERIFY_IS_TRUE(s_testVMCallback->CheckDisplayValues(wstring(L"12345678"), wstring(L"27217528.63236")));
+        VERIFY_IS_TRUE(s_testVMCallback->CheckDisplayValues(wstring(L"12345678"), wstring(L"27217529")));
     }
 
     // Test large values