Bug 487614 - KCalc does not consistently follow the order of operations, leading to incorrect results for certain expressions (e.g., 8 - 3 * 2 + 1 evaluates to 1 instead of 3).
Summary: KCalc does not consistently follow the order of operations, leading to incorr...
Status: RESOLVED FIXED
Alias: None
Product: kcalc
Classification: Applications
Component: general (show other bugs)
Version: 24.05.0
Platform: Arch Linux Linux
: NOR major
Target Milestone: ---
Assignee: Evan Teran
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-05-27 08:09 UTC by dargaard
Modified: 2024-05-28 16:27 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description dargaard 2024-05-27 08:09:20 UTC
KCalc does not consistently follow the order of operations, leading to incorrect results for certain expressions (e.g., 8 - 3 * 2 + 1 evaluates to 1 instead of 3).

STEPS TO REPRODUCE
I can reproduce this behavior with any question that has addition, subtraction, and multiplication. I have evaluated other calculations and get correct answers however questions that specifically have 3 steps which include addition, subtraction and multiplication generate incorrect answers, such as my example 8-3*2+1 evaluates to 1 instead of 3.

OBSERVED RESULT
8-3*2+1=1

EXPECTED RESULT
8-3*2+1=3

SOFTWARE/OS VERSIONS
Kernel: 6.9.2-zen1-1-zen
KDE Plasma Version: Plasma 6.0.5
KDE Frameworks Version:  6.2.0
Qt Version:  6.7.1

ADDITIONAL INFORMATION
Just to make sure I wasn't making a human error I did the same test calculations using a physical calculator, and the android calculator, both of which gave correct answers, I also ran this by chat gpt just to see if it could find a mistake I was making and it agreed that Kcalc is giving incorrect answers.
Comment 1 Antonio Rojas 2024-05-27 09:37:28 UTC
Both this and bug 487566 are caused by https://invent.kde.org/utilities/kcalc/-/commit/c7864e9547b2ef2e9e318a88c78ba51f10fbc5e3

A calculator giving wrong answers is pretty bad and makes it quite useless, so i'd suggest reverting this if it can't be fixed soon.
Comment 2 Gabriel Barrantes 2024-05-27 18:52:26 UTC
(In reply to Antonio Rojas from comment #1)
> Both this and bug 487566 are caused by
> https://invent.kde.org/utilities/kcalc/-/commit/
> c7864e9547b2ef2e9e318a88c78ba51f10fbc5e3
> 
> A calculator giving wrong answers is pretty bad and makes it quite useless,
> so i'd suggest reverting this if it can't be fixed soon.
Let me take a look.
Comment 3 Bug Janitor Service 2024-05-27 22:18:19 UTC
A possibly relevant merge request was started @ https://invent.kde.org/utilities/kcalc/-/merge_requests/89
Comment 4 Gabriel Barrantes 2024-05-27 22:22:52 UTC
(In reply to dargaard from comment #0)
> KCalc does not consistently follow the order of operations, leading to
> incorrect results for certain expressions (e.g., 8 - 3 * 2 + 1 evaluates to
> 1 instead of 3).
> 
> STEPS TO REPRODUCE
> I can reproduce this behavior with any question that has addition,
> subtraction, and multiplication. I have evaluated other calculations and get
> correct answers however questions that specifically have 3 steps which
> include addition, subtraction and multiplication generate incorrect answers,
> such as my example 8-3*2+1 evaluates to 1 instead of 3.
> 
> OBSERVED RESULT
> 8-3*2+1=1
> 
> EXPECTED RESULT
> 8-3*2+1=3
> 
> SOFTWARE/OS VERSIONS
> Kernel: 6.9.2-zen1-1-zen
> KDE Plasma Version: Plasma 6.0.5
> KDE Frameworks Version:  6.2.0
> Qt Version:  6.7.1
> 
> ADDITIONAL INFORMATION
> Just to make sure I wasn't making a human error I did the same test
> calculations using a physical calculator, and the android calculator, both
> of which gave correct answers, I also ran this by chat gpt just to see if it
> could find a mistake I was making and it agreed that Kcalc is giving
> incorrect answers.

The - operator has been tricky because sometimes acts as as binary (the ordinary subtraction) and other times is unary (inverting the sign of the number that is next to it), I submitted a patch and hopefully no more issues will arise.
If any other input gives wrong results let me know, for sure we could use some more testing since the feature is fairly new.
Comment 5 Gabriel Barrantes 2024-05-27 23:54:31 UTC
Git commit 91205962fa89d0a31abb21a2aaee9aff17d36b91 by Gabriel Barrantes.
Committed on 27/05/2024 at 22:14.
Pushed by gabrielbarrantes into branch 'master'.

Add edge case handler for stack reduction

Inputs with the form 8-3*2+1 reduced incorrectly due to
the "+" operation dropping the "-" for the first operand.
Added code to detect this special case and give the expected result.

M  +20   -0    autotests/kcalc_parser_core_test.cpp
M  +27   -0    kcalc_core.cpp
M  +10   -0    kcalc_token.cpp
M  +4    -0    kcalc_token.h

https://invent.kde.org/utilities/kcalc/-/commit/91205962fa89d0a31abb21a2aaee9aff17d36b91
Comment 6 Antonio Rojas 2024-05-28 16:27:25 UTC
Thanks for the fix, please push to the 24.05 branch too.