Skip to content

Commit

Permalink
Fixup C++ warnings
Browse files Browse the repository at this point in the history 
Check for exactly 1 before using the singular noun, rather than greater than 1

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 Jun 3, 2022
1 parent 8fab2cb commit 254d71c
Show file tree
Hide file tree
Showing 16 changed files with 144 additions and 167 deletions.
27 changes: 14 additions & 13 deletions src/CalcManager/CEngine/CalcInput.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,17 +108,16 @@ bool CalcInput::TryAddDigit(unsigned int value, uint32_t radix, bool isIntegerMo

if (radix == 8)
{
switch (wordBitWidth % 3)
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:
}
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;
}
}
else if (radix == 10)
Expand Down Expand Up @@ -253,15 +252,17 @@ 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;
}
}

Expand Down
11 changes: 5 additions & 6 deletions src/CalcManager/CEngine/History.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ void CHistoryCollector::EnclosePrecInversionBrackets()

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

// AddUnaryOpToHistory
Expand Down Expand Up @@ -330,10 +330,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 @@ -356,7 +355,7 @@ void CHistoryCollector::ClearHistoryLine(wstring_view errStr)
{
if (errStr.empty()) // in case of error let the display stay as it is
{
if (nullptr != m_pCalcDisplay)
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>>>());
Expand Down Expand Up @@ -412,7 +411,7 @@ void CHistoryCollector::TruncateEquationSzFromIch(int ich)
// Adds the m_pszEquation into the running history text
void CHistoryCollector::SetExpressionDisplay()
{
if (nullptr != m_pCalcDisplay)
if (m_pCalcDisplay != nullptr)
{
m_pCalcDisplay->SetExpressionDisplay(m_spTokens, m_spCommands);
}
Expand Down
8 changes: 4 additions & 4 deletions src/CalcManager/CEngine/Rational.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace CalcEngine

Rational::Rational(int32_t i)
{
PRAT pr = i32torat(static_cast<int32_t>(i));
PRAT pr = i32torat(i);

m_p = Number{ pr->pp };
m_q = Number{ pr->pq };
Expand All @@ -42,7 +42,7 @@ namespace CalcEngine

Rational::Rational(uint32_t ui)
{
PRAT pr = Ui32torat(static_cast<uint32_t>(ui));
PRAT pr = Ui32torat(ui);

m_p = Number{ pr->pp };
m_q = Number{ pr->pq };
Expand All @@ -52,8 +52,8 @@ namespace CalcEngine

Rational::Rational(uint64_t ui)
{
uint32_t hi = (uint32_t)(((ui) >> 32) & 0xffffffff);
uint32_t lo = (uint32_t)ui;
uint32_t hi = static_cast<uint32_t>(ui >> 32);
uint32_t lo = static_cast<uint32_t>(ui & 0xffffffff);

Rational temp = (Rational{ hi } << 32) | lo;

Expand Down
16 changes: 4 additions & 12 deletions src/CalcManager/CEngine/calc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ void CCalcEngine::InitChopNumbers()
assert(m_chopNumbers.size() == m_maxDecimalValueStrings.size());
for (size_t i = 0; i < m_chopNumbers.size(); i++)
{
auto maxVal = m_chopNumbers[i] / 2;
auto maxVal = m_chopNumbers[i] >> 1;
maxVal = RationalMath::Integer(maxVal);

m_maxDecimalValueStrings[i] = maxVal.ToString(10, NumberFormat::Float, m_precision);
Expand Down Expand Up @@ -170,14 +170,6 @@ void CCalcEngine::SettingsChanged()
wstring grpStr = m_resourceProvider->GetCEngineString(L"sGrouping");
m_decGrouping = DigitGroupingStringToGroupingVector(grpStr.empty() ? DEFAULT_GRP_STR : grpStr);

bool numChanged = false;

// if the grouping pattern or thousands symbol changed we need to refresh the display
if (m_decGrouping != lastDecGrouping || m_groupSeparator != lastSep)
{
numChanged = true;
}

// if the decimal symbol has changed we always do the following things
if (m_decimalSeparator != lastDec)
{
Expand All @@ -189,10 +181,10 @@ void CCalcEngine::SettingsChanged()
s_engineStrings[SIDS_DECIMAL_SEPARATOR] = m_decimalSeparator;

// we need to redraw to update the decimal point button
numChanged = true;
DisplayNum();
}

if (numChanged)
// if the grouping pattern or thousands symbol changed we need to refresh the display
else if (m_decGrouping != lastDecGrouping || m_groupSeparator != lastSep)
{
DisplayNum();
}
Expand Down
25 changes: 12 additions & 13 deletions src/CalcManager/CEngine/scicomm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@ namespace
// 0 is returned. Higher the number, higher the precedence of the operator.
int NPrecedenceOfOp(int nopCode)
{
static uint16_t rgbPrec[] = { 0, 0, IDC_OR, 0, IDC_XOR, 0, IDC_AND, 1, IDC_NAND, 1, IDC_NOR, 1, IDC_ADD, 2, IDC_SUB, 2, IDC_RSHF, 3,
IDC_LSHF, 3, IDC_RSHFL, 3, IDC_MOD, 3, IDC_DIV, 3, IDC_MUL, 3, IDC_PWR, 4, IDC_ROOT, 4, IDC_LOGBASEY, 4 };
static const int rgbPrec[][2] = { { 0, 0 }, { IDC_OR, 0 }, { IDC_XOR, 0 }, { IDC_AND, 1 }, { IDC_NAND, 1 }, { IDC_NOR, 1 },
{ IDC_ADD, 2 }, { IDC_SUB, 2 }, { IDC_RSHF, 3 }, { IDC_LSHF, 3 }, { IDC_RSHFL, 3 }, { IDC_MOD, 3 },
{ IDC_DIV, 3 }, { IDC_MUL, 3 }, { IDC_PWR, 4 }, { IDC_ROOT, 4 }, { IDC_LOGBASEY, 4 } };

for (unsigned int iPrec = 0; iPrec < size(rgbPrec); iPrec += 2)
for (size_t iPrec = 0; iPrec < size(rgbPrec); iPrec++)
{
if (nopCode == rgbPrec[iPrec])
if (rgbPrec[iPrec][0] == nopCode)
{
return rgbPrec[iPrec + 1];
return rgbPrec[iPrec][1];
}
}
return 0;
Expand All @@ -59,7 +60,7 @@ void CCalcEngine::HandleErrorCommand(OpCode idc)

void CCalcEngine::HandleMaxDigitsReached()
{
if (nullptr != m_pCalcDisplay)
if (m_pCalcDisplay != nullptr)
{
m_pCalcDisplay->MaxDigitsReached();
}
Expand All @@ -77,7 +78,7 @@ void CCalcEngine::ClearTemporaryValues()

void CCalcEngine::ClearDisplay()
{
if (nullptr != m_pCalcDisplay)
if (m_pCalcDisplay != nullptr)
{
m_pCalcDisplay->SetExpressionDisplay(make_shared<vector<pair<wstring, int>>>(), make_shared<vector<shared_ptr<IExpressionCommand>>>());
}
Expand Down Expand Up @@ -396,7 +397,7 @@ void CCalcEngine::ProcessCommandWorker(OpCode wParam)

/* clear the parenthesis status box indicator, this will not be
cleared for CENTR */
if (nullptr != m_pCalcDisplay)
if (m_pCalcDisplay != nullptr)
{
m_pCalcDisplay->SetParenthesisNumber(0);
ClearDisplay();
Expand Down Expand Up @@ -518,7 +519,7 @@ void CCalcEngine::ProcessCommandWorker(OpCode wParam)

if (wParam == IDC_OPENP)
{
// if there's an omitted multiplication sign
// if there's an omitted multiplication sign
if (IsDigitOpCode(m_nLastCom) || IsUnaryOpCode(m_nLastCom) || m_nLastCom == IDC_PNT || m_nLastCom == IDC_CLOSEP)
{
ProcessCommand(IDC_MUL);
Expand Down Expand Up @@ -587,17 +588,15 @@ void CCalcEngine::ProcessCommandWorker(OpCode wParam)
m_HistoryCollector.AddCloseBraceToHistory();

// Now get back the operation and opcode at the beginning of this parenthesis pair

m_openParenCount -= 1;
m_lastVal = m_parenVals[m_openParenCount];
m_lastVal = m_parenVals[--m_openParenCount];
m_nOpCode = m_nOp[m_openParenCount];

// m_bChangeOp should be true if m_nOpCode is valid
m_bChangeOp = (m_nOpCode != 0);
}

// Set the "(=xx" indicator.
if (nullptr != m_pCalcDisplay)
if (m_pCalcDisplay != nullptr)
{
m_pCalcDisplay->SetParenthesisNumber(static_cast<unsigned int>(m_openParenCount));
}
Expand Down
5 changes: 3 additions & 2 deletions src/CalcManager/CEngine/scidisp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,18 @@ CalcEngine::Rational CCalcEngine::TruncateNumForIntMath(CalcEngine::Rational con

// Truncate to an integer. Do not round here.
auto result = RationalMath::Integer(rat);
const auto chopNumber = GetChopNumber();

// Can be converting a dec negative number to Hex/Oct/Bin rep. Use 2's complement form
// Check the range.
if (result < 0)
{
// if negative make positive by doing a twos complement
result = -(result)-1;
result ^= GetChopNumber();
result ^= chopNumber;
}

result &= GetChopNumber();
result &= chopNumber;

return result;
}
Expand Down
2 changes: 1 addition & 1 deletion src/CalcManager/CEngine/scifunc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ CalcEngine::Rational CCalcEngine::SciCalcFunctions(CalcEngine::Rational const& r
result = Integer(rat);

uint64_t w64Bits = result.ToUInt64_t();
uint64_t lsb = ((w64Bits & 0x01) == 1) ? 1 : 0;
uint64_t lsb = w64Bits & 1;
w64Bits >>= 1; // RShift by 1

if (op == IDC_ROR)
Expand Down
37 changes: 18 additions & 19 deletions src/CalcManager/CEngine/scioper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,12 @@ CalcEngine::Rational CCalcEngine::DoOperation(int operation, CalcEngine::Rationa
if (fMsb)
{
result = Integer(result);
const auto chopNumber = GetChopNumber();

auto tempRat = GetChopNumber() >> holdVal;
auto tempRat = chopNumber >> holdVal;
tempRat = Integer(tempRat);

result |= tempRat ^ GetChopNumber();
result |= tempRat ^ chopNumber;
}
break;
}
Expand Down Expand Up @@ -94,7 +95,7 @@ CalcEngine::Rational CCalcEngine::DoOperation(int operation, CalcEngine::Rationa
case IDC_DIV:
case IDC_MOD:
{
int iNumeratorSign = 1, iDenominatorSign = 1;
bool isNumeratorNegative = false, isDenominatorNegative = false;
auto temp = result;
result = rhs;

Expand All @@ -107,7 +108,7 @@ CalcEngine::Rational CCalcEngine::DoOperation(int operation, CalcEngine::Rationa
{
result = (rhs ^ GetChopNumber()) + 1;

iNumeratorSign = -1;
isNumeratorNegative = true;
}

w64Bits = temp.ToUInt64_t();
Expand All @@ -117,36 +118,34 @@ CalcEngine::Rational CCalcEngine::DoOperation(int operation, CalcEngine::Rationa
{
temp = (temp ^ GetChopNumber()) + 1;

iDenominatorSign = -1;
isDenominatorNegative = true;
}
}

if (operation == IDC_DIV)
{
result /= temp;
if (m_fIntegerMode && (iNumeratorSign * iDenominatorSign) == -1)
if (m_fIntegerMode && (isNumeratorNegative != isDenominatorNegative))
{
result = -(Integer(result));
}
}
else
else if (m_fIntegerMode)
{
if (m_fIntegerMode)
{
// Programmer mode, use remrat (remainder after division)
result %= temp;
// Programmer mode, use remrat (remainder after division)
result %= temp;

if (iNumeratorSign == -1)
{
result = -(Integer(result));
}
}
else
if (isNumeratorNegative)
{
// other modes, use modrat (modulus after division)
result = Mod(result, temp);
result = -(Integer(result));
}
}
else
{
// other modes, use modrat (modulus after division)
result = Mod(result, temp);
}

break;
}

Expand Down
4 changes: 1 addition & 3 deletions src/CalcManager/CEngine/sciset.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ void CCalcEngine::SetRadixTypeAndNumWidth(RadixType radixtype, NUM_WIDTH numwidt
if (fMsb)
{
// If high bit is set, then get the decimal number in -ve 2'scompl form.
auto tempResult = m_currentVal ^ GetChopNumber();

m_currentVal = -(tempResult + 1);
m_currentVal = -((m_currentVal ^ GetChopNumber()) + 1);
}
}

Expand Down
Loading

0 comments on commit 254d71c

Please sign in to comment.