Skip to content

Commit

Permalink
Fix C++ warnings
Browse files Browse the repository at this point in the history 
Check for exactly 1 before using the singular noun, rather than check for greater than 1, just in case somehow the variable being evaluated ends up as 0

In addition, should this code ever be refactored in such a way that the check to 0 is changed, this will ensure the end result does not change. In addition, it is naturally immediately apparent to the developers what is going on, potentially enhancing maintainability.

Additionally, make bitwise operations more obvious in their intentions.
  • Loading branch information
@AZero13
AZero13 committed Sep 5, 2022
1 parent f27bdba commit c74b411
Show file tree
Hide file tree
Showing 80 changed files with 1,286 additions and 1,350 deletions.
59 changes: 30 additions & 29 deletions src/CalcManager/CEngine/CalcInput.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ bool CalcInput::TryToggleSign(bool isIntegerMode, wstring_view maxNumStr)
return true;
}

bool CalcInput::TryAddDigit(unsigned int value, uint32_t radix, bool isIntegerMode, wstring_view maxNumStr, int32_t wordBitWidth, int maxDigits)
bool CalcInput::TryAddDigit(unsigned int value, uint32_t radix, bool isIntegerMode, wstring_view maxNumStr, uint32_t wordBitWidth, int maxDigits)
{
// Convert from an integer into a character
// This includes both normal digits and alpha 'digits' for radixes > 10
auto chDigit = static_cast<wchar_t>((value < 10) ? (L'0' + value) : (L'A' + value - 10));
auto chDigit = static_cast<wchar_t>(value < 10 ? L'0' + value : L'A' + value - 10);

CalcNumSec* pNumSec;
size_t maxCount;
Expand Down Expand Up @@ -89,7 +89,7 @@ bool CalcInput::TryAddDigit(unsigned int value, uint32_t radix, bool isIntegerMo
}

// Ignore leading zeros
if (pNumSec->IsEmpty() && (value == 0))
if (pNumSec->IsEmpty() && value == 0)
{
return true;
}
Expand All @@ -108,17 +108,16 @@ bool CalcInput::TryAddDigit(unsigned int value, uint32_t radix, bool isIntegerMo

if (radix == 8)
{
switch (wordBitWidth % 3)
const auto modResult = wordBitWidth % 3;
if (modResult == 1)
{
case 1:
// in 16 or 64bit word size, if the first digit is a 1 we can enter 6 (16bit) or 22 (64bit) digits
allowExtraDigit = (pNumSec->value.front() == L'1');
break;

case 2:
allowExtraDigit = pNumSec->value.front() == L'1';
}
else if (modResult == 2)
{
// in 8 or 32bit word size, if the first digit is a 3 or less we can enter 3 (8bit) or 11 (32bit) digits
allowExtraDigit = (pNumSec->value.front() <= L'3');
break;
allowExtraDigit = pNumSec->value.front() <= L'3';
}
}
else if (radix == 10)
Expand All @@ -137,14 +136,14 @@ bool CalcInput::TryAddDigit(unsigned int value, uint32_t radix, bool isIntegerMo

// If cmpResult == 0:
// Undecided still. The case when max is "127", and current number is "12". Look for the new number being 7 or less to allow
auto cmpResult = pNumSec->value.compare(0, wstring::npos, maxNumStr, 0, pNumSec->value.size());
const auto cmpResult = pNumSec->value.compare(0, wstring::npos, maxNumStr, 0, pNumSec->value.size());
if (cmpResult < 0)
{
allowExtraDigit = true;
}
else if (cmpResult == 0)
{
auto lastChar = maxNumStr[pNumSec->value.size()];
const auto lastChar = maxNumStr[pNumSec->value.size()];
if (chDigit <= lastChar)
{
allowExtraDigit = true;
Expand Down Expand Up @@ -189,7 +188,7 @@ bool CalcInput::TryAddDecimalPt()
return true;
}

bool CalcInput::HasDecimalPt()
bool CalcInput::HasDecimalPt() const
{
return m_hasDecimal;
}
Expand Down Expand Up @@ -253,29 +252,31 @@ void CalcInput::Backspace()

void CalcInput::SetDecimalSymbol(wchar_t decSymbol)
{
if (m_decSymbol != decSymbol)
if (m_decSymbol == decSymbol)
{
m_decSymbol = decSymbol;
return;
}

if (m_hasDecimal)
{
// Change to new decimal pt
m_base.value[m_decPtIndex] = m_decSymbol;
}
m_decSymbol = decSymbol;

if (m_hasDecimal)
{
// Change to new decimal pt
m_base.value[m_decPtIndex] = m_decSymbol;
}
}

bool CalcInput::IsEmpty()
bool CalcInput::IsEmpty() const
{
return m_base.IsEmpty() && !m_hasExponent && m_exponent.IsEmpty() && !m_hasDecimal;
}

wstring CalcInput::ToString(uint32_t radix)
wstring CalcInput::ToString(uint32_t radix) const
{
// In theory both the base and exponent could be C_NUM_MAX_DIGITS long.
if ((m_base.value.size() > MAX_STRLEN) || (m_hasExponent && m_exponent.value.size() > MAX_STRLEN))
if (m_base.value.size() > MAX_STRLEN || m_hasExponent && m_exponent.value.size() > MAX_STRLEN)
{
return wstring();
return {};
}

wstring result;
Expand All @@ -302,8 +303,8 @@ wstring CalcInput::ToString(uint32_t radix)
result += m_decSymbol;
}

result += ((radix == 10) ? L'e' : L'^');
result += (m_exponent.IsNegative() ? L'-' : L'+');
result += radix == 10 ? L'e' : L'^';
result += m_exponent.IsNegative() ? L'-' : L'+';

if (m_exponent.IsEmpty())
{
Expand All @@ -318,13 +319,13 @@ wstring CalcInput::ToString(uint32_t radix)
// Base and Exp can each be up to C_NUM_MAX_DIGITS in length, plus 4 characters for sign, dec, exp, and expSign.
if (result.size() > C_NUM_MAX_DIGITS * 2 + 4)
{
return wstring();
return {};
}

return result;
}

Rational CalcInput::ToRational(uint32_t radix, int32_t precision)
Rational CalcInput::ToRational(uint32_t radix, int32_t precision) const
{
PRAT rat = StringToRat(m_base.IsNegative(), m_base.value, m_exponent.IsNegative(), m_exponent.value, radix, precision);
if (rat == nullptr)
Expand Down
10 changes: 5 additions & 5 deletions src/CalcManager/CEngine/CalcUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

bool IsOpInRange(OpCode op, uint32_t x, uint32_t y)
{
return ((op >= x) && (op <= y));
return op >= x && op <= y;
}

bool IsBinOpCode(OpCode opCode)
Expand All @@ -18,7 +18,7 @@ bool IsBinOpCode(OpCode opCode)
// of it and catch it themselves or not needing this
bool IsUnaryOpCode(OpCode opCode)
{
return (IsOpInRange(opCode, IDC_UNARYFIRST, IDC_UNARYLAST) || IsOpInRange(opCode, IDC_UNARYEXTENDEDFIRST, IDC_UNARYEXTENDEDLAST));
return IsOpInRange(opCode, IDC_UNARYFIRST, IDC_UNARYLAST) || IsOpInRange(opCode, IDC_UNARYEXTENDEDFIRST, IDC_UNARYEXTENDEDLAST);
}

bool IsDigitOpCode(OpCode opCode)
Expand Down Expand Up @@ -49,8 +49,8 @@ bool IsGuiSettingOpCode(OpCode opCode)
case IDC_MPLUS:
case IDC_MMINUS:
return true;
default:
// most of the commands
return false;
}

// most of the commands
return false;
}
78 changes: 38 additions & 40 deletions src/CalcManager/CEngine/History.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ using namespace CalcEngine;
namespace
{
template <typename T>
static void Truncate(vector<T>& v, unsigned int index)
void Truncate(vector<T>& v, unsigned int index)
{
if (index >= v.size())
{
Expand Down Expand Up @@ -45,7 +45,7 @@ void CHistoryCollector::ReinitHistory()
// Constructor
// Can throw Out of memory error
CHistoryCollector::CHistoryCollector(ICalcDisplay* pCalcDisplay, std::shared_ptr<IHistoryDisplay> pHistoryDisplay, wchar_t decimalSymbol)
: m_pHistoryDisplay(pHistoryDisplay)
: m_pHistoryDisplay(std::move(pHistoryDisplay))
, m_pCalcDisplay(pCalcDisplay)
, m_iCurLineHistStart(-1)
, m_decimalSymbol(decimalSymbol)
Expand All @@ -66,13 +66,13 @@ CHistoryCollector::~CHistoryCollector()

void CHistoryCollector::AddOpndToHistory(wstring_view numStr, Rational const& rat, bool fRepetition)
{
std::shared_ptr<std::vector<int>> commands = std::make_shared<vector<int>>();
auto commands = std::make_shared<vector<int>>();
// Check for negate
bool fNegative = (numStr[0] == L'-');
bool fNegative = numStr[0] == L'-';
bool fSciFmt = false;
bool fDecimal = false;

for (size_t i = (fNegative ? 1 : 0); i < numStr.length(); i++)
for (size_t i = fNegative ? 1 : 0; i < numStr.length(); i++)
{
if (numStr[i] == m_decimalSymbol)
{
Expand All @@ -98,8 +98,7 @@ void CHistoryCollector::AddOpndToHistory(wstring_view numStr, Rational const& ra
// Number
else
{
int num = static_cast<int>(numStr[i]) - ASCII_0;
num += IDC_0;
int num = static_cast<int>(numStr[i]) - ASCII_0 + IDC_0;
commands->push_back(num);
}
}
Expand Down Expand Up @@ -143,19 +142,19 @@ void CHistoryCollector::AddBinOpToHistory(int nOpCode, bool isIntegerMode, bool
// This is expected to be called when a binary op in the last say 1+2+ is changing to another one say 1+2* (+ changed to *)
// It needs to know by this change a Precedence inversion happened. i.e. previous op was lower or equal to its previous op, but the new
// one isn't. (Eg. 1*2* to 1*2^). It can add explicit brackets to ensure the precedence is inverted. (Eg. (1*2) ^)
void CHistoryCollector::ChangeLastBinOp(int nOpCode, bool fPrecInvToHigher, bool isIntgerMode)
void CHistoryCollector::ChangeLastBinOp(int nOpCode, bool fPrecInvToHigher, bool isIntegerMode)
{
TruncateEquationSzFromIch(m_lastBinOpStartIndex);
if (fPrecInvToHigher)
{
EnclosePrecInversionBrackets();
}
AddBinOpToHistory(nOpCode, isIntgerMode);
AddBinOpToHistory(nOpCode, isIntegerMode);
}

void CHistoryCollector::PushLastOpndStart(int ichOpndStart)
{
int ich = (ichOpndStart == -1) ? m_lastOpStartIndex : ichOpndStart;
int ich = ichOpndStart == -1 ? m_lastOpStartIndex : ichOpndStart;

if (m_curOperandIndex < static_cast<int>(m_operandIndices.size()))
{
Expand All @@ -174,8 +173,7 @@ void CHistoryCollector::PopLastOpndStart()
void CHistoryCollector::AddOpenBraceToHistory()
{
AddCommand(std::make_shared<CParentheses>(IDC_OPENP));
int ichOpndStart = IchAddSzToEquationSz(CCalcEngine::OpCodeToString(IDC_OPENP), -1);
PushLastOpndStart(ichOpndStart);
PushLastOpndStart(IchAddSzToEquationSz(CCalcEngine::OpCodeToString(IDC_OPENP), -1));

SetExpressionDisplay();
m_lastBinOpStartIndex = -1;
Expand All @@ -195,15 +193,15 @@ void CHistoryCollector::AddCloseBraceToHistory()
void CHistoryCollector::EnclosePrecInversionBrackets()
{
// Top of the Opnd starts index or 0 is nothing is in top
int ichStart = (m_curOperandIndex > 0) ? m_operandIndices[m_curOperandIndex - 1] : 0;
int ichStart = m_curOperandIndex > 0 ? m_operandIndices[m_curOperandIndex - 1] : 0;

InsertSzInEquationSz(CCalcEngine::OpCodeToString(IDC_OPENP), -1, ichStart);
IchAddSzToEquationSz(CCalcEngine::OpCodeToString(IDC_CLOSEP), -1);
}

bool CHistoryCollector::FOpndAddedToHistory()
bool CHistoryCollector::FOpndAddedToHistory() const
{
return (-1 != m_lastOpStartIndex);
return m_lastOpStartIndex != -1;
}

// AddUnaryOpToHistory
Expand Down Expand Up @@ -244,7 +242,7 @@ void CHistoryCollector::AddUnaryOpToHistory(int nOpCode, bool fInv, AngleType an
angleOpCode = CalculationManager::Command::CommandGRAD;
}

int command = nOpCode;
int command;
switch (nOpCode)
{
case IDC_SIN:
Expand Down Expand Up @@ -330,10 +328,9 @@ void CHistoryCollector::AddUnaryOpToHistory(int nOpCode, bool fInv, AngleType an
// history of equations
void CHistoryCollector::CompleteHistoryLine(wstring_view numStr)
{
if (nullptr != m_pHistoryDisplay)
if (m_pHistoryDisplay != nullptr)
{
unsigned int addedItemIndex = m_pHistoryDisplay->AddToHistory(m_spTokens, m_spCommands, numStr);
m_pCalcDisplay->OnHistoryItemAdded(addedItemIndex);
m_pCalcDisplay->OnHistoryItemAdded(m_pHistoryDisplay->AddToHistory(m_spTokens, m_spCommands, numStr));
}

m_spTokens = nullptr;
Expand All @@ -354,16 +351,18 @@ void CHistoryCollector::CompleteEquation(std::wstring_view numStr)

void CHistoryCollector::ClearHistoryLine(wstring_view errStr)
{
if (errStr.empty()) // in case of error let the display stay as it is
if (!errStr.empty()) // in case of error let the display stay as it is
{
if (nullptr != m_pCalcDisplay)
{
m_pCalcDisplay->SetExpressionDisplay(
std::make_shared<std::vector<std::pair<std::wstring, int>>>(), std::make_shared<std::vector<std::shared_ptr<IExpressionCommand>>>());
}
m_iCurLineHistStart = -1; // It will get recomputed at the first Opnd
ReinitHistory();
return;
}

if (m_pCalcDisplay != nullptr)
{
m_pCalcDisplay->SetExpressionDisplay(
std::make_shared<std::vector<std::pair<std::wstring, int>>>(), std::make_shared<std::vector<std::shared_ptr<IExpressionCommand>>>());
}
m_iCurLineHistStart = -1; // It will get recomputed at the first Opnd
ReinitHistory();
}

// Adds the given string psz to the globally maintained current equation string at the end.
Expand All @@ -380,25 +379,25 @@ int CHistoryCollector::IchAddSzToEquationSz(wstring_view str, int icommandIndex)
}

// Inserts a given string into the global m_pszEquation at the given index ich taking care of reallocations etc.
void CHistoryCollector::InsertSzInEquationSz(wstring_view str, int icommandIndex, int ich)
void CHistoryCollector::InsertSzInEquationSz(wstring_view str, int icommandIndex, int ich) const
{
m_spTokens->emplace(m_spTokens->begin() + ich, wstring(str), icommandIndex);
}

// Chops off the current equation string from the given index
void CHistoryCollector::TruncateEquationSzFromIch(int ich)
void CHistoryCollector::TruncateEquationSzFromIch(int ich) const
{
// Truncate commands
int minIdx = -1;
unsigned int nTokens = static_cast<unsigned int>(m_spTokens->size());
auto nTokens = m_spTokens->size();

for (unsigned int i = ich; i < nTokens; i++)
for (size_t i = ich; i < nTokens; i++)
{
const auto& currentPair = (*m_spTokens)[i];
int curTokenId = currentPair.second;
const int curTokenId = currentPair.second;
if (curTokenId != -1)
{
if ((minIdx != -1) || (curTokenId < minIdx))
if (minIdx != -1 || curTokenId < minIdx)
{
minIdx = curTokenId;
Truncate(*m_spCommands, minIdx);
Expand All @@ -410,9 +409,9 @@ void CHistoryCollector::TruncateEquationSzFromIch(int ich)
}

// Adds the m_pszEquation into the running history text
void CHistoryCollector::SetExpressionDisplay()
void CHistoryCollector::SetExpressionDisplay() const
{
if (nullptr != m_pCalcDisplay)
if (m_pCalcDisplay != nullptr)
{
m_pCalcDisplay->SetExpressionDisplay(m_spTokens, m_spCommands);
}
Expand Down Expand Up @@ -465,13 +464,13 @@ void CHistoryCollector::SetDecimalSymbol(wchar_t decimalSymbol)
}

// Update the commands corresponding to the passed string Number
std::shared_ptr<std::vector<int>> CHistoryCollector::GetOperandCommandsFromString(wstring_view numStr)
std::shared_ptr<std::vector<int>> CHistoryCollector::GetOperandCommandsFromString(wstring_view numStr) const
{
std::shared_ptr<std::vector<int>> commands = std::make_shared<std::vector<int>>();
// Check for negate
bool fNegative = (numStr[0] == L'-');
bool fNegative = numStr[0] == L'-';

for (size_t i = (fNegative ? 1 : 0); i < numStr.length(); i++)
for (size_t i = fNegative ? 1 : 0; i < numStr.length(); i++)
{
if (numStr[i] == m_decimalSymbol)
{
Expand All @@ -492,8 +491,7 @@ std::shared_ptr<std::vector<int>> CHistoryCollector::GetOperandCommandsFromStrin
// Number
else
{
int num = static_cast<int>(numStr[i]) - ASCII_0;
num += IDC_0;
int num = static_cast<int>(numStr[i]) - ASCII_0 + IDC_0;
commands->push_back(num);
}
}
Expand Down
Loading

0 comments on commit c74b411

Please sign in to comment.