From 08f4d90356d935a1d147698ba3e4fbc5ca73aed6 Mon Sep 17 00:00:00 2001 From: Mateusz Piesta Date: Wed, 8 Feb 2023 07:40:39 +0100 Subject: [PATCH] [MOS-530] Various calculator app fixes * Fixed issue with subtract operation triggering unusal behavior. * Unified handling of the add,sub,mul,div operations. * Updated and verified existing unit tests and added new ones covering unhandled cases. * Added basic requirements/readme. --- .../data/CalculatorInputProcessor.cpp | 8 -- .../data/CalculatorInputProcessorText.cpp | 48 +++----- .../data/CalculatorInputProcessorText.hpp | 4 +- .../data/CalculatorUtility.cpp | 9 +- module-apps/application-calculator/readme.md | 23 ++++ .../tests/CalculatorInput_tests.cpp | 110 +++++++++++++----- .../rotator/include/rotator/Rotator.hpp | 2 +- pure_changelog.md | 1 + 8 files changed, 131 insertions(+), 74 deletions(-) create mode 100644 module-apps/application-calculator/readme.md diff --git a/module-apps/application-calculator/data/CalculatorInputProcessor.cpp b/module-apps/application-calculator/data/CalculatorInputProcessor.cpp index baa297351124779a312e123856b6cf5c008b266f..d3d38f70f595520f926b67a0b98c369c362a63a9 100644 --- a/module-apps/application-calculator/data/CalculatorInputProcessor.cpp +++ b/module-apps/application-calculator/data/CalculatorInputProcessor.cpp @@ -9,15 +9,10 @@ bool calc::InputProcessor::isSymbol(uint32_t code) noexcept switch (code) { case plus: - [[fallthrough]]; case minus: - [[fallthrough]]; case division: - [[fallthrough]]; case multiplication: - [[fallthrough]]; case comma: - [[fallthrough]]; case full_stop: return true; default: @@ -31,11 +26,8 @@ bool calc::InputProcessor::isOperation(uint32_t code) noexcept switch (code) { case plus: - [[fallthrough]]; case minus: - [[fallthrough]]; case division: - [[fallthrough]]; case multiplication: return true; default: diff --git a/module-apps/application-calculator/data/CalculatorInputProcessorText.cpp b/module-apps/application-calculator/data/CalculatorInputProcessorText.cpp index 7a8a87816e548ea3f7c0e6391ba375cf658bd32c..0d64b6b9a55766cbde1ecbdc843948c9091d356b 100644 --- a/module-apps/application-calculator/data/CalculatorInputProcessorText.cpp +++ b/module-apps/application-calculator/data/CalculatorInputProcessorText.cpp @@ -13,7 +13,7 @@ calc::InputProcessorText::InputProcessorText(gsl::strict_not_null i bool calc::InputProcessorText::handle(const gui::InputEvent &event) { - if (clearInput || inputContainsExponent()) { + if (clearInput) { clear(); } @@ -90,7 +90,7 @@ bool calc::InputProcessorText::handle(const gui::InputEvent &event) return true; } - if (prohibidInput(event)) { + if (prohibitInput(event)) { // Consume event to don't allow more decimals return true; } @@ -119,13 +119,18 @@ std::optional calc::InputProcessorText::lastCharacter() const bool calc::InputProcessorText::lastCharacterIsSymbol() const { const auto &c = lastCharacter(); - return c ? isSymbol(*c) : false; + return c && isSymbol(*c); } bool calc::InputProcessorText::lastCharacterIsOperation() const { const auto &c = lastCharacter(); - return c ? isOperation(*c) : false; + return c && isOperation(*c); +} + +bool calc::InputProcessorText::isThereOnlyOneChar() const +{ + return inputField->getText().length() == 1; } std::optional calc::InputProcessorText::penultimateCharacter() const @@ -143,13 +148,13 @@ std::optional calc::InputProcessorText::penultimateCharacter() const bool calc::InputProcessorText::penultimateCharacterIsSymbol() const { const auto &c = penultimateCharacter(); - return c ? isSymbol(*c) : false; + return c && isSymbol(*c); } bool calc::InputProcessorText::penultimateCharacterIsDecimalSeparator() const { const auto &c = penultimateCharacter(); - return c ? isDecimalSeparator(*c) : false; + return c && isDecimalSeparator(*c); } bool calc::InputProcessorText::shouldComputeBeforeNextOperation() const @@ -178,7 +183,7 @@ void calc::InputProcessorText::addSymbol(const UTF8 &symbol) { if (!inputField->getText().empty()) { - if (lastCharacterIsSymbol() && symbol != symbols::strings::minus) { + if (lastCharacterIsSymbol()) { if (!penultimateCharacterIsSymbol() && inputField->getText().length() > 1) { inputField->removeChar(); inputField->addText(symbol); @@ -195,19 +200,7 @@ void calc::InputProcessorText::addSymbol(const UTF8 &symbol) bool calc::InputProcessorText::shouldHideInput(const gui::InputEvent &event) const { - if (!lastCharacterIsOperation()) { - return false; - } - - if (!event.isDigit() && !event.is(gui::KeyCode::KEY_DOWN)) { - return false; - } - - if (inputField->getText() == symbols::strings::minus) { - return false; - } - - return true; + return not isThereOnlyOneChar() and lastCharacterIsOperation() and event.isDigit(); } bool calc::InputProcessorText::shouldRestoreInput(const gui::InputEvent &event) const @@ -263,12 +256,7 @@ bool calc::InputProcessorText::isCurrentNumberDecimal() const return false; } -bool calc::InputProcessorText::inputContainsExponent() const -{ - return std::string{inputField->getText()}.find('e') != std::string::npos; -} - -bool calc::InputProcessorText::prohibidInput(const gui::InputEvent &event) const +bool calc::InputProcessorText::prohibitInput(const gui::InputEvent &event) const { if (!event.isDigit()) { return false; @@ -295,16 +283,12 @@ bool calc::InputProcessorText::decimalLimitReached() const const auto &txt = std::string{inputField->getText()}; const auto separator_pos = txt.find_last_of(symbols::strings::decimal_separator_str()); - if ((txt.size() - separator_pos) > limits::MaxDecimalDigits) { - return true; - } - - return false; + return (txt.size() - separator_pos) > limits::MaxDecimalDigits; } void calc::InputProcessorText::compute() { - auto result = Calculator().calculate(hiddenPartOfEquation + inputField->getText()); + const auto result = Calculator().calculate(hiddenPartOfEquation + inputField->getText()); inputField->setText(result.value); hiddenPartOfEquation.clear(); clearInput = result.isError; diff --git a/module-apps/application-calculator/data/CalculatorInputProcessorText.hpp b/module-apps/application-calculator/data/CalculatorInputProcessorText.hpp index fb621a41fa890734c001e18072135c941d721292..921c08e9de6e7986fa504835bd5e08895f35ba03 100644 --- a/module-apps/application-calculator/data/CalculatorInputProcessorText.hpp +++ b/module-apps/application-calculator/data/CalculatorInputProcessorText.hpp @@ -25,6 +25,7 @@ namespace calc std::optional lastCharacter() const; bool lastCharacterIsSymbol() const; bool lastCharacterIsOperation() const; + bool isThereOnlyOneChar() const; std::optional penultimateCharacter() const; bool penultimateCharacterIsSymbol() const; @@ -41,9 +42,8 @@ namespace calc bool hasHiddenPart() const; bool isCurrentNumberDecimal() const; - bool inputContainsExponent() const; - bool prohibidInput(const gui::InputEvent &event) const; + bool prohibitInput(const gui::InputEvent &event) const; bool charactedLimitReached() const; bool decimalLimitReached() const; diff --git a/module-apps/application-calculator/data/CalculatorUtility.cpp b/module-apps/application-calculator/data/CalculatorUtility.cpp index 7dc97ac8cf802c1ccfba937d9407e36e4335a95c..b2bb76583a517f1b01a0b15e46f52ac70bd8ec1d 100644 --- a/module-apps/application-calculator/data/CalculatorUtility.cpp +++ b/module-apps/application-calculator/data/CalculatorUtility.cpp @@ -22,7 +22,7 @@ namespace calc } int error; - double result = te_interp(source.c_str(), &error); + const auto result = te_interp(source.c_str(), &error); if (error == 0 && !std::isinf(result) && !std::isnan(result)) { auto output = utils::to_string(result); if (output.length() > MaxStringLength) { @@ -49,8 +49,9 @@ namespace calc size_t index = 0; while (true) { index = input.find(from, index); - if (index == std::string::npos) + if (index == std::string::npos) { break; + } input.replace(index, from.length(), to); index += to.length(); } @@ -61,8 +62,8 @@ namespace calc { using namespace calc::limits; - auto base = static_cast(result); - auto length = utils::to_string(base).length(); + const auto base = static_cast(result); + auto length = utils::to_string(base).length(); if (base < 0) { length -= 1; } diff --git a/module-apps/application-calculator/readme.md b/module-apps/application-calculator/readme.md new file mode 100644 index 0000000000000000000000000000000000000000..58fc44510dde883b5d6728f5bdc3bcddbbc2ade8 --- /dev/null +++ b/module-apps/application-calculator/readme.md @@ -0,0 +1,23 @@ +# Assumptions + +* It's not possible to type numbers with `+` prefix like `+5` +* It's not possible to type operation like `5--5`. Instead, use `5+5` +* Dividing by 0 will result in displaying `Error` message +* Input is limited to the 7 characters including +* Maximal input value can be set to `9999999` or `99999.9` +* Minimal input value can be set to `-999999` or `-9999.9` +* Press `*` to remove the last typed input. It can be used multiple times to revert more than one input value. +* It is possible to recall the last typed operation by using the `*` key. First you have to clear the current input, and + then press the `*` once again. + +``` +123+ +322 <- start pressing `*` +32 +3 + <- empty input field, press the `*` once again +123+ +``` + +* Trying to execute operations without right operand will trigger displaying `Error` message, i.e. `100+=` +* There is no support for typing values using scientific notation, i.e. `10e9` diff --git a/module-apps/application-calculator/tests/CalculatorInput_tests.cpp b/module-apps/application-calculator/tests/CalculatorInput_tests.cpp index 5b9a022fdc6952a9ef937e93ee2727ff0d07e46f..95676cdf9c636634c46878299182d6e5f0bf1c73 100644 --- a/module-apps/application-calculator/tests/CalculatorInput_tests.cpp +++ b/module-apps/application-calculator/tests/CalculatorInput_tests.cpp @@ -47,13 +47,15 @@ SCENARIO("Input Processor tests") auto passLongKeyPress = [&](KeyCodes code) { passEvent(longPressEvent(code)); }; auto passShortKeyPresses = [&](const std::vector &codes) { - for (const auto &code : codes) + for (const auto &code : codes) { passShortKeyPress(code); + } }; auto passLongKeyPresses = [&](const std::vector &codes) { - for (const auto &code : codes) + for (const auto &code : codes) { passLongKeyPress(code); + } }; THEN("The text is empty") @@ -260,27 +262,6 @@ SCENARIO("Input Processor tests") } } - WHEN("We enter a negative number") - { - passShortKeyPresses({MinusKey, KeyCodes::NumericKey2, KeyCodes::NumericKey3}); - - THEN("It is shown") - { - REQUIRE(inputField.getText() == "-23"); - } - - AND_WHEN("We try to subtract a negitive number") - { - passShortKeyPresses( - {MinusKey, MinusKey, KeyCodes::NumericKey1, KeyCodes::NumericKey3, KeyCodes::JoystickEnter}); - - THEN("The result is computed properly") - { - REQUIRE(inputField.getText() == "-10"); - } - } - } - WHEN("We enter a number") { passShortKeyPresses({KeyCodes::NumericKey1, KeyCodes::NumericKey2, KeyCodes::NumericKey3}); @@ -332,9 +313,9 @@ SCENARIO("Input Processor tests") { passShortKeyPresses({MinusKey, KeyCodes::NumericKey5, KeyCodes::NumericKey6}); - THEN("Previous input is hidden and negative number is shown") + THEN("Previous input is hidden and typed number is shown") { - REQUIRE(inputField.getText() == "-56"); + REQUIRE(inputField.getText() == "56"); } AND_WHEN("We press enter") @@ -349,7 +330,7 @@ SCENARIO("Input Processor tests") AND_WHEN("We delete the input") { - passShortKeyPresses({3, KeyCodes::NumericKeyPnd}); + passShortKeyPresses({2, KeyCodes::NumericKeyPnd}); THEN("Input is deleted") { @@ -362,7 +343,7 @@ SCENARIO("Input Processor tests") THEN("Previous input is restored") { - REQUIRE(inputField.getText() == "123+"); + REQUIRE(inputField.getText() == "123-"); } } } @@ -478,6 +459,26 @@ SCENARIO("Input Processor tests") REQUIRE(inputField.getText() == "56"); } } + + AND_WHEN("We invoke any operation") + { + passShortKeyPress(KeyCodes::JoystickUp); + + THEN("We should see") + { + REQUIRE(inputField.getText() == "9.9998e9+"); + } + + AND_WHEN("We proceed to type more input") + { + passShortKeyPresses({KeyCodes::NumericKey1, KeyCodes::NumericKey5, KeyCodes::NumericKey6}); + + THEN("We should see") + { + REQUIRE(inputField.getText() == "156"); + } + } + } } WHEN("We enter an equation") @@ -508,5 +509,60 @@ SCENARIO("Input Processor tests") } } } + + WHEN("We enter the minus key") + { + passShortKeyPress(MinusKey); + + AND_WHEN("We press another minus") + { + passShortKeyPress(MinusKey); + + THEN("We should see only one minus sign") + { + REQUIRE(inputField.getText() == "-"); + } + + AND_WHEN("We continue inputting minus sign") + { + passShortKeyPress(MinusKey); + + THEN("We still should see only one minus sign") + { + REQUIRE(inputField.getText() == "-"); + } + } + } + + AND_WHEN("We input a number") + { + passShortKeyPress(KeyCodes::NumericKey5); + + THEN("We should see a '-5' value") + { + REQUIRE(inputField.getText() == "-5"); + } + + AND_WHEN("We input another minus sign") + { + passShortKeyPress(MinusKey); + + THEN("We should see a '-5-' value") + { + REQUIRE(inputField.getText() == "-5-"); + } + + AND_WHEN("We input another minus sign") + { + passShortKeyPress(MinusKey); + + THEN("We should still see a '-5-' value") + { + REQUIRE(inputField.getText() == "-5-"); + } + } + } + } + } } } diff --git a/module-utils/rotator/include/rotator/Rotator.hpp b/module-utils/rotator/include/rotator/Rotator.hpp index 8a8f654de09a6543b6bd2cfaf754a19abfbbe84c..4b5c8d88bb122a644c7d933564b6d7db6f9fce08 100644 --- a/module-utils/rotator/include/rotator/Rotator.hpp +++ b/module-utils/rotator/include/rotator/Rotator.hpp @@ -44,7 +44,7 @@ namespace utils return path; } - uint getFileNumber(const std::string &filename) const + std::size_t getFileNumber(const std::string &filename) const { const auto position = filename.rfind("."); if (position == std::string::npos) { diff --git a/pure_changelog.md b/pure_changelog.md index 5e79e334d8f1e70749ce6b34deafdc30726055de..c592cf36b97e2b34ed251aef439f74454829054a 100644 --- a/pure_changelog.md +++ b/pure_changelog.md @@ -71,6 +71,7 @@ * Fixed screen ghosting after emoji selection * Fixed incorrect loudspeaker icon in call window when headset was connected during a call * Fixed invalid screen displayed after missed call +* Fixed minor issues in the Calculator Application ## [1.5.0 2022-12-20]