Darkempire78 / OpenCalc

A simple and beautiful calculator for Android
GNU General Public License v3.0
737 stars 82 forks source link

Percent Calculation doesn't work #116

Closed Uniquely6958 closed 1 year ago

Uniquely6958 commented 1 year ago

When evaluating a percent, it should consider the value immediately preceding the '%' sign to be divided by 100. So 100 * 95% should equal 95.

However, entering that into the calculator, we get 9,500, or a straight multiplication. First

More interestingly, for larger values, the calculation gets even more bizarre. So 1700 * 95% should be 1,615. But the actual result is 2,745,500, where 2,745,500 = 1,700 * 1,700 * 95 Second

GEROGIANNIS commented 1 year ago

Hello there , i can not replicate the bug you are descibing.

Screenshot_20221207-134735_Calculator_1.png

Screenshot_20221207-134815_Calculator_1.png

What version of opencalc are you using?

Uniquely6958 commented 1 year ago

It's v2.0.0-beta3 from fdroid.

GEROGIANNIS commented 1 year ago

It's v2.0.0-beta3 from fdroid.

Im using v1.8.3 also from fdroid but i do not seem to have the bug 🤔

Uniquely6958 commented 1 year ago

I got the same result after downgrading to v1.8.3 on fdroid so it isn't version specific.

I'm using Samsung on OneUI 4.1 with Android 12. Would that have anything to do with it?

My apologies. Fdroid hadn't allowed me to install the older version and kept me on the latest, which I missed.

I can now confirm that I am getting the same results that you are on v1.8.3.

Additionally, I tried the same on v2.0.0-beta2 and the bug appeared again. So the bug lies between this and the prior build.

Uniquely6958 commented 1 year ago

The offending commit seems to be https://github.com/Darkempire78/OpenCalc/commit/9d26dd7ebefce4aecebca6c884fcc2945ddc96e0.

Running the following: getPercentString("100*95%") returns (100)*(100)*(95)%, which gets further transformed into (100)*(100)*(95)/100, explaining the result.

I'm not an Android developer so what I said might be completely wrong.

ac87 commented 1 year ago

@Darkempire78 I have created a branch https://github.com/Darkempire78/OpenCalc/commit/4a6be0fecb446625f438b76583679b902da0584a that I don't plan to PR, Maths is one of my biggest weaknesses.

However as you said you are new to Android development I think its useful to give you an idea of how unit tests work. I think you'd benefit greatly from having unit tests for the calculations.

Darkempire78 commented 1 year ago

@Darkempire78 I have created a branch 4a6be0f that I don't plan to PR, Maths is one of my biggest weaknesses.

However as you said you are new to Android development I think its useful to give you an idea of how unit tests work. I think you'd benefit greatly from having unit tests for the calculations.

Thanks, I will add unit test during the week

mizzunet commented 1 year ago

Facing the same issue here on v2.0.0 beta3

Darkempire78 commented 1 year ago

It should be fixed in the v2.0.0-beta7 (https://github.com/Darkempire78/OpenCalc/releases/tag/v2.0.0-beta7)