DevWiki/calculator
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix invalid check of m_precedenceOpCount (#298)

Browse Source 
The conditional m_precedenceOpCount >= 0 was always true because m_precendenceOpCount is an unsigned type. Update the conditional to simply be true and rely on a break statement in the loop. Although this member variable used to be a signed type, in practice, the value was never less than 0.

How changes were validated:
Manual. Unit tests pass locally.
This commit is contained in:
Daniel Belcher 2019-04-09 14:12:28 -07:00 committed by GitHub
parent af41a183a7
commit 7ac750f7e5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 52 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

89
src/CalcManager/CEngine/scicomm.cpp
View File

@ -463,45 +463,12 @@ void CCalcEngine::ProcessCommandWorker(OpCode wParam)
m_HistoryCollector.AddOpndToHistory(m_numberString, m_currentVal);
}
do {
if (m_nOpCode) /* Is there a valid operation around? */
// Evaluate the precedence stack.
ResolveHighestPrecedenceOperation();
while (m_fPrecedence && m_precedenceOpCount > 0)
{
/* If this is the first EQU in a string, set m_holdVal=m_currentVal */
/* Otherwise let m_currentVal=m_holdVal. This keeps m_currentVal constant */
/* through all EQUs in a row. */
if (m_bNoPrevEqu)
{
m_holdVal = m_currentVal;
}
else
{
m_currentVal = m_holdVal;
DisplayNum(); // to update the m_numberString
m_HistoryCollector.AddBinOpToHistory(m_nOpCode, false);
m_HistoryCollector.AddOpndToHistory(m_numberString, m_currentVal); // Adding the repeated last op to history
}
// Do the current or last operation.
m_currentVal = DoOperation(m_nOpCode, m_currentVal, m_lastVal);
m_nPrevOpCode = m_nOpCode;
m_lastVal = m_currentVal;
/* Check for errors. If this wasn't done, DisplayNum */
/* would immediately overwrite any error message. */
if (!m_bError)
DisplayNum();
/* No longer the first EQU. */
m_bNoPrevEqu = false;
}
else if (!m_bError)
DisplayNum();
if (m_precedenceOpCount == 0 || !m_fPrecedence)
break;
m_nOpCode = m_nPrecOp[--m_precedenceOpCount];
m_precedenceOpCount--;
m_nOpCode = m_nPrecOp[m_precedenceOpCount];
m_lastVal = m_precedenceVals[m_precedenceOpCount];
// Precedence Inversion check
@ -514,7 +481,9 @@ void CCalcEngine::ProcessCommandWorker(OpCode wParam)
m_HistoryCollector.PopLastOpndStart();
m_bNoPrevEqu = true;
} while (m_precedenceOpCount >= 0);
ResolveHighestPrecedenceOperation();
}
if (!m_bError)
{
@ -789,6 +758,48 @@ void CCalcEngine::ProcessCommandWorker(OpCode wParam)
}
// Helper function to resolve one item on the precedence stack.
void CCalcEngine::ResolveHighestPrecedenceOperation()
{
// Is there a valid operation around?
if (m_nOpCode)
{
// If this is the first EQU in a string, set m_holdVal=m_currentVal
// Otherwise let m_currentVal=m_holdVal. This keeps m_currentVal constant
// through all EQUs in a row.
if (m_bNoPrevEqu)
{
m_holdVal = m_currentVal;
}
else
{
m_currentVal = m_holdVal;
DisplayNum(); // to update the m_numberString
m_HistoryCollector.AddBinOpToHistory(m_nOpCode, false);
m_HistoryCollector.AddOpndToHistory(m_numberString, m_currentVal); // Adding the repeated last op to history
}
// Do the current or last operation.
m_currentVal = DoOperation(m_nOpCode, m_currentVal, m_lastVal);
m_nPrevOpCode = m_nOpCode;
m_lastVal = m_currentVal;
// Check for errors. If this wasn't done, DisplayNum
// would immediately overwrite any error message.
if (!m_bError)
{
DisplayNum();
}
// No longer the first EQU.
m_bNoPrevEqu = false;
}
else if (!m_bError)
{
DisplayNum();
}
}
// CheckAndAddLastBinOpToHistory
//
// This is a very confusing helper routine to add the last entered binary operator to the history. This is expected to

1
src/CalcManager/Header Files/CalcEngine.h
View File

@ -129,6 +129,7 @@ private:
private:
void ProcessCommandWorker(OpCode wParam);
void ResolveHighestPrecedenceOperation();
void HandleErrorCommand(OpCode idc);
void HandleMaxDigitsReached();
void DisplayNum(void);