From 4883fab7f7200e73fb12a7b3057d41400dcb0477 Mon Sep 17 00:00:00 2001 From: Josh Koon <45607479+joshkoon@users.noreply.github.com> Date: Mon, 28 Jan 2019 19:14:15 -0800 Subject: [PATCH 1/4] Convert ExpressionCommand and History collector to use Rational instead of PRAT --- src/CalcManager/CEngine/History.cpp | 6 ++-- src/CalcManager/CEngine/scicomm.cpp | 29 +++++-------------- src/CalcManager/ExpressionCommand.cpp | 40 ++++++++------------------ src/CalcManager/ExpressionCommand.h | 13 +++++---- src/CalcManager/Header Files/History.h | 3 +- 5 files changed, 31 insertions(+), 60 deletions(-) diff --git a/src/CalcManager/CEngine/History.cpp b/src/CalcManager/CEngine/History.cpp index 092b4d9a..b2ba1c14 100644 --- a/src/CalcManager/CEngine/History.cpp +++ b/src/CalcManager/CEngine/History.cpp @@ -12,6 +12,7 @@ constexpr int ASCII_0 = 48; using namespace std; +using namespace CalcEngine; void CHistoryCollector::ReinitHistory() { @@ -51,7 +52,7 @@ CHistoryCollector::~CHistoryCollector() } } -void CHistoryCollector::AddOpndToHistory(wstring_view numStr, PRAT hNoNum, bool fRepetition) +void CHistoryCollector::AddOpndToHistory(wstring_view numStr, Rational rat, bool fRepetition) { std::shared_ptr> commands = std::make_shared>(); // Check for negate @@ -91,8 +92,7 @@ void CHistoryCollector::AddOpndToHistory(wstring_view numStr, PRAT hNoNum, bool } } - auto operandCommand = std::make_shared(commands, fNegative, fDecimal, fSciFmt); - operandCommand->Initialize(hNoNum); + auto operandCommand = std::make_shared(commands, rat, fNegative, fDecimal, fSciFmt); int iCommandEnd = AddCommand(operandCommand); m_lastOpStartIndex = IchAddSzToEquationSz(numStr, iCommandEnd); diff --git a/src/CalcManager/CEngine/scicomm.cpp b/src/CalcManager/CEngine/scicomm.cpp index 354fb37d..eea137b4 100644 --- a/src/CalcManager/CEngine/scicomm.cpp +++ b/src/CalcManager/CEngine/scicomm.cpp @@ -211,9 +211,7 @@ void CCalcEngine::ProcessCommandWorker(WPARAM wParam) if (!m_HistoryCollector.FOpndAddedToHistory()) { // if the prev command was ) or unop then it is already in history as a opnd form (...) - PRAT curRat = m_currentVal.ToPRAT(); - m_HistoryCollector.AddOpndToHistory(m_numberString, curRat); - destroyrat(curRat); + m_HistoryCollector.AddOpndToHistory(m_numberString, m_currentVal); } /* m_bChangeOp is true if there was an operation done and the */ @@ -310,10 +308,7 @@ DoPrecedenceCheckAgain: { if (!m_HistoryCollector.FOpndAddedToHistory()) { - - PRAT curRat = m_currentVal.ToPRAT(); - m_HistoryCollector.AddOpndToHistory(m_numberString, curRat); - destroyrat(curRat); + m_HistoryCollector.AddOpndToHistory(m_numberString, m_currentVal); } m_HistoryCollector.AddUnaryOpToHistory((INT)wParam, m_bInv, m_angletype); @@ -339,9 +334,7 @@ DoPrecedenceCheckAgain: if(wParam == IDC_PERCENT) { CheckAndAddLastBinOpToHistory(); - PRAT curRat = m_currentVal.ToPRAT(); - m_HistoryCollector.AddOpndToHistory(m_numberString, curRat, true); - destroyrat(curRat); + m_HistoryCollector.AddOpndToHistory(m_numberString, m_currentVal, true); } /* reset the m_bInv flag and indicators if it is set @@ -464,9 +457,7 @@ DoPrecedenceCheckAgain: if (!m_HistoryCollector.FOpndAddedToHistory()) { - PRAT curRat = m_currentVal.ToPRAT(); - m_HistoryCollector.AddOpndToHistory(m_numberString, curRat); - destroyrat(curRat); + m_HistoryCollector.AddOpndToHistory(m_numberString, m_currentVal); } do { @@ -485,9 +476,7 @@ DoPrecedenceCheckAgain: m_currentVal = m_holdVal; DisplayNum(); // to update the m_numberString m_HistoryCollector.AddBinOpToHistory(m_nOpCode, false); - PRAT curRat = m_currentVal.ToPRAT(); - m_HistoryCollector.AddOpndToHistory(m_numberString, curRat); // Adding the repeated last op to history - destroyrat(curRat); + m_HistoryCollector.AddOpndToHistory(m_numberString, m_currentVal); // Adding the repeated last op to history } // Do the current or last operation. @@ -595,9 +584,7 @@ DoPrecedenceCheckAgain: if (!m_HistoryCollector.FOpndAddedToHistory()) { - PRAT curRat = m_currentVal.ToPRAT(); - m_HistoryCollector.AddOpndToHistory(m_numberString, curRat); - destroyrat(curRat); + m_HistoryCollector.AddOpndToHistory(m_numberString, m_currentVal); } // Get the operation and number and return result. @@ -701,9 +688,7 @@ DoPrecedenceCheckAgain: if (!m_HistoryCollector.FOpndAddedToHistory()) { - PRAT curRat = m_currentVal.ToPRAT(); - m_HistoryCollector.AddOpndToHistory(m_numberString, curRat); - destroyrat(curRat); + m_HistoryCollector.AddOpndToHistory(m_numberString, m_currentVal); } PRAT curRat = m_currentVal.ToPRAT(); diff --git a/src/CalcManager/ExpressionCommand.cpp b/src/CalcManager/ExpressionCommand.cpp index ec177a64..c8766177 100644 --- a/src/CalcManager/ExpressionCommand.cpp +++ b/src/CalcManager/ExpressionCommand.cpp @@ -7,6 +7,7 @@ #include "ExpressionCommand.h" using namespace std; +using namespace CalcEngine; constexpr wchar_t chNegate = L'-'; constexpr wchar_t chExp = L'e'; @@ -95,25 +96,12 @@ void CBinaryCommand::Accept(_In_ ISerializeCommandVisitor &commandVisitor) } COpndCommand::COpndCommand(_In_ shared_ptr> const &commands, - _In_ bool fNegative, - _In_ bool fDecimal, - _In_ bool fSciFmt) : - m_commands(commands), m_fNegative(fNegative), m_fDecimal(fDecimal), m_fSciFmt(fSciFmt) -{ - m_hnoNum = nullptr; -} - - -void COpndCommand::Initialize(_In_ PRAT hNoNum) -{ - assert(&m_hnoNum != nullptr); - if (m_hnoNum != nullptr) - { - destroyrat(m_hnoNum); - m_hnoNum = nullptr; - } - DUPRAT(m_hnoNum, hNoNum); -} + Rational const& rat, + bool fNegative, + bool fDecimal, + bool fSciFmt) : + m_commands(commands), m_value{ rat }, m_fNegative(fNegative), m_fDecimal(fDecimal), m_fSciFmt(fSciFmt) +{} const shared_ptr> & COpndCommand::GetCommands() const { @@ -294,19 +282,15 @@ const wstring & COpndCommand::GetToken(wchar_t decimalSymbol) wstring COpndCommand::GetString(uint32_t radix, int32_t precision, wchar_t decimalSymbol) { - wstring numString{}; - if (m_hnoNum != nullptr) - { - numString = NumObjToString(m_hnoNum, radix, eNUMOBJ_FMT::FMT_FLOAT, precision); - } + PRAT valRat = m_value.ToPRAT(); + auto result = NumObjToString(valRat, radix, eNUMOBJ_FMT::FMT_FLOAT, precision); + destroyrat(valRat); - return numString; + return result; } COpndCommand::~COpndCommand() -{ - destroyrat(m_hnoNum); -} +{} void COpndCommand::Accept(_In_ ISerializeCommandVisitor &commandVisitor) { diff --git a/src/CalcManager/ExpressionCommand.h b/src/CalcManager/ExpressionCommand.h index d36c51ab..ffdb0435 100644 --- a/src/CalcManager/ExpressionCommand.h +++ b/src/CalcManager/ExpressionCommand.h @@ -3,7 +3,8 @@ #pragma once #include "ExpressionCommandInterface.h" -#include "Header Files\CalcEngine.h" +#include "Header Files/CalcEngine.h" +#include "Header Files/Rational.h" class CParentheses : public IParenthesisCommand { @@ -49,11 +50,11 @@ class COpndCommand : public IOpndCommand { public: COpndCommand(_In_ std::shared_ptr> const &commands, - _In_ bool fNegative, - _In_ bool fDecimal, - _In_ bool fSciFmt); + CalcEngine::Rational const& rat, + bool fNegative, + bool fDecimal, + bool fSciFmt); ~COpndCommand(); - void Initialize(_In_ PRAT hNoNum); const std::shared_ptr> & GetCommands() const; void SetCommands(std::shared_ptr> const& commands); @@ -74,7 +75,7 @@ private: bool m_fSciFmt; bool m_fDecimal; std::wstring m_token; - PRAT m_hnoNum; + CalcEngine::Rational m_value; void ClearAllAndAppendCommand(CalculationManager::Command command); }; diff --git a/src/CalcManager/Header Files/History.h b/src/CalcManager/Header Files/History.h index f885067f..b084375d 100644 --- a/src/CalcManager/Header Files/History.h +++ b/src/CalcManager/Header Files/History.h @@ -5,6 +5,7 @@ #include "ICalcDisplay.h" #include "IHistoryDisplay.h" +#include "Rational.h" // maximum depth you can get by precedence. It is just an array's size limit. static constexpr size_t MAXPRECDEPTH = 25; @@ -16,7 +17,7 @@ class CHistoryCollector { public: CHistoryCollector(ICalcDisplay *pCalcDisplay, std::shared_ptr pHistoryDisplay, wchar_t decimalSymbol); // Can throw errors ~CHistoryCollector(); - void AddOpndToHistory(std::wstring_view numStr, PRAT hNoNum, bool fRepetition = false); + void AddOpndToHistory(std::wstring_view numStr, CalcEngine::Rational rat, bool fRepetition = false); void RemoveLastOpndFromHistory(); void AddBinOpToHistory(int nOpCode, bool fNoRepetition = true); void ChangeLastBinOp(int nOpCode, bool fPrecInvToHigher); From db4a6eb9ea5dbd213eb75b28d6eed4c9464af5ae Mon Sep 17 00:00:00 2001 From: Josh Koon <45607479+joshkoon@users.noreply.github.com> Date: Mon, 28 Jan 2019 19:34:36 -0800 Subject: [PATCH 2/4] Return to initialization pattern in ExpressionCommand --- src/CalcManager/CEngine/History.cpp | 7 ++++--- src/CalcManager/ExpressionCommand.cpp | 28 +++++++++++++++++--------- src/CalcManager/ExpressionCommand.h | 5 +++-- src/CalcManager/Header Files/History.h | 2 +- 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/CalcManager/CEngine/History.cpp b/src/CalcManager/CEngine/History.cpp index b2ba1c14..95347a59 100644 --- a/src/CalcManager/CEngine/History.cpp +++ b/src/CalcManager/CEngine/History.cpp @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. #include "pch.h" @@ -52,7 +52,7 @@ CHistoryCollector::~CHistoryCollector() } } -void CHistoryCollector::AddOpndToHistory(wstring_view numStr, Rational rat, bool fRepetition) +void CHistoryCollector::AddOpndToHistory(wstring_view numStr, Rational const& rat, bool fRepetition) { std::shared_ptr> commands = std::make_shared>(); // Check for negate @@ -92,7 +92,8 @@ void CHistoryCollector::AddOpndToHistory(wstring_view numStr, Rational rat, bool } } - auto operandCommand = std::make_shared(commands, rat, fNegative, fDecimal, fSciFmt); + auto operandCommand = std::make_shared(commands, fNegative, fDecimal, fSciFmt); + operandCommand->Initialize(rat); int iCommandEnd = AddCommand(operandCommand); m_lastOpStartIndex = IchAddSzToEquationSz(numStr, iCommandEnd); diff --git a/src/CalcManager/ExpressionCommand.cpp b/src/CalcManager/ExpressionCommand.cpp index c8766177..07c4cf7c 100644 --- a/src/CalcManager/ExpressionCommand.cpp +++ b/src/CalcManager/ExpressionCommand.cpp @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. #include "pch.h" @@ -96,13 +96,18 @@ void CBinaryCommand::Accept(_In_ ISerializeCommandVisitor &commandVisitor) } COpndCommand::COpndCommand(_In_ shared_ptr> const &commands, - Rational const& rat, - bool fNegative, - bool fDecimal, - bool fSciFmt) : - m_commands(commands), m_value{ rat }, m_fNegative(fNegative), m_fDecimal(fDecimal), m_fSciFmt(fSciFmt) + bool fNegative, + bool fDecimal, + bool fSciFmt) : + m_commands(commands), m_fNegative(fNegative), m_fDecimal(fDecimal), m_fSciFmt(fSciFmt), m_fInitialized(false), m_value{} {} +void COpndCommand::Initialize(Rational const& rat) +{ + m_value = rat; + m_fInitialized = true; +} + const shared_ptr> & COpndCommand::GetCommands() const { return m_commands; @@ -282,9 +287,14 @@ const wstring & COpndCommand::GetToken(wchar_t decimalSymbol) wstring COpndCommand::GetString(uint32_t radix, int32_t precision, wchar_t decimalSymbol) { - PRAT valRat = m_value.ToPRAT(); - auto result = NumObjToString(valRat, radix, eNUMOBJ_FMT::FMT_FLOAT, precision); - destroyrat(valRat); + wstring result{}; + + if (m_fInitialized) + { + PRAT valRat = m_value.ToPRAT(); + result = NumObjToString(valRat, radix, eNUMOBJ_FMT::FMT_FLOAT, precision); + destroyrat(valRat); + } return result; } diff --git a/src/CalcManager/ExpressionCommand.h b/src/CalcManager/ExpressionCommand.h index ffdb0435..10bed355 100644 --- a/src/CalcManager/ExpressionCommand.h +++ b/src/CalcManager/ExpressionCommand.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 @@ -50,11 +50,11 @@ class COpndCommand : public IOpndCommand { public: COpndCommand(_In_ std::shared_ptr> const &commands, - CalcEngine::Rational const& rat, bool fNegative, bool fDecimal, bool fSciFmt); ~COpndCommand(); + void Initialize(CalcEngine::Rational const& rat); const std::shared_ptr> & GetCommands() const; void SetCommands(std::shared_ptr> const& commands); @@ -74,6 +74,7 @@ private: bool m_fNegative; bool m_fSciFmt; bool m_fDecimal; + bool m_fInitialized; std::wstring m_token; CalcEngine::Rational m_value; void ClearAllAndAppendCommand(CalculationManager::Command command); diff --git a/src/CalcManager/Header Files/History.h b/src/CalcManager/Header Files/History.h index b084375d..5deb5fff 100644 --- a/src/CalcManager/Header Files/History.h +++ b/src/CalcManager/Header Files/History.h @@ -17,7 +17,7 @@ class CHistoryCollector { public: CHistoryCollector(ICalcDisplay *pCalcDisplay, std::shared_ptr pHistoryDisplay, wchar_t decimalSymbol); // Can throw errors ~CHistoryCollector(); - void AddOpndToHistory(std::wstring_view numStr, CalcEngine::Rational rat, bool fRepetition = false); + void AddOpndToHistory(std::wstring_view numStr, CalcEngine::Rational const& rat, bool fRepetition = false); void RemoveLastOpndFromHistory(); void AddBinOpToHistory(int nOpCode, bool fNoRepetition = true); void ChangeLastBinOp(int nOpCode, bool fPrecInvToHigher); From b70a12c6cf6972e8ebfe798cd8edcbae603f3f46 Mon Sep 17 00:00:00 2001 From: Josh Koon <45607479+joshkoon@users.noreply.github.com> Date: Tue, 29 Jan 2019 14:27:24 -0800 Subject: [PATCH 3/4] Add clarifying comment to call to CHistoryCollector::AddOpndToHistory --- src/CalcManager/CEngine/scicomm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CalcManager/CEngine/scicomm.cpp b/src/CalcManager/CEngine/scicomm.cpp index eea137b4..7dbbf02d 100644 --- a/src/CalcManager/CEngine/scicomm.cpp +++ b/src/CalcManager/CEngine/scicomm.cpp @@ -334,7 +334,7 @@ DoPrecedenceCheckAgain: if(wParam == IDC_PERCENT) { CheckAndAddLastBinOpToHistory(); - m_HistoryCollector.AddOpndToHistory(m_numberString, m_currentVal, true); + m_HistoryCollector.AddOpndToHistory(m_numberString, m_currentVal, true /* Add to primary and secondary display */); } /* reset the m_bInv flag and indicators if it is set From ebfce5a8cd368ba5eb7740b9c70e98f57b382f06 Mon Sep 17 00:00:00 2001 From: Josh Koon <45607479+joshkoon@users.noreply.github.com> Date: Tue, 29 Jan 2019 14:28:03 -0800 Subject: [PATCH 4/4] Remove empty COpndCommand destructor. Cleanup constructor declaration. --- src/CalcManager/ExpressionCommand.cpp | 15 +++++++-------- src/CalcManager/ExpressionCommand.h | 4 ++-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/CalcManager/ExpressionCommand.cpp b/src/CalcManager/ExpressionCommand.cpp index 07c4cf7c..e671182e 100644 --- a/src/CalcManager/ExpressionCommand.cpp +++ b/src/CalcManager/ExpressionCommand.cpp @@ -95,11 +95,13 @@ void CBinaryCommand::Accept(_In_ ISerializeCommandVisitor &commandVisitor) commandVisitor.Visit(*this); } -COpndCommand::COpndCommand(_In_ shared_ptr> const &commands, - bool fNegative, - bool fDecimal, - bool fSciFmt) : - m_commands(commands), m_fNegative(fNegative), m_fDecimal(fDecimal), m_fSciFmt(fSciFmt), m_fInitialized(false), m_value{} +COpndCommand::COpndCommand(shared_ptr> const &commands, bool fNegative, bool fDecimal, bool fSciFmt) : + m_commands(commands), + m_fNegative(fNegative), + m_fDecimal(fDecimal), + m_fSciFmt(fSciFmt), + m_fInitialized(false), + m_value{} {} void COpndCommand::Initialize(Rational const& rat) @@ -299,9 +301,6 @@ wstring COpndCommand::GetString(uint32_t radix, int32_t precision, wchar_t decim return result; } -COpndCommand::~COpndCommand() -{} - void COpndCommand::Accept(_In_ ISerializeCommandVisitor &commandVisitor) { commandVisitor.Visit(*this); diff --git a/src/CalcManager/ExpressionCommand.h b/src/CalcManager/ExpressionCommand.h index 10bed355..0953ab26 100644 --- a/src/CalcManager/ExpressionCommand.h +++ b/src/CalcManager/ExpressionCommand.h @@ -49,11 +49,11 @@ private: class COpndCommand : public IOpndCommand { public: - COpndCommand(_In_ std::shared_ptr> const &commands, + COpndCommand( + std::shared_ptr> const &commands, bool fNegative, bool fDecimal, bool fSciFmt); - ~COpndCommand(); void Initialize(CalcEngine::Rational const& rat); const std::shared_ptr> & GetCommands() const;