search

Darkempire78 / OpenCalc

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

#314 fix: issue with factorial decimal number #429

Closed muryno closed 1 month ago

muryno commented 2 months ago

Fixes #314

The issue arose because the original code overlooked decimal points when identifying numbers leading up to a factorial symbol, mishandling cases like "3.0!". By adding a check for decimals, the updated code now properly handles these numbers

Before

old1 old2

MR result

Screenshot 2024-02-19 at 21 37 33
inson1 commented 2 months ago

@muryno its wrong, '.' doesnt have to be always '.', it matters on your languague

this is used for it "DecimalFormatSymbols.getInstance().decimalSeparator.toString()" and in main activity there is decimalSeparatorSymbol variable

muryno commented 2 months ago

DecimalFormatSymbols @inson1 I saw that but if you try to debug the cause for why 50.0 ! is not equals 50!

it is because the . is ignored and this function private fun formatFactorial(calculation: String): String { always return case like factorial(50(factorial(.0)) instead of factorial(50.0)

adding the check in this PR fix the issue..

I also added a test case to validate this. you can try to put a break point at formatFactorial and see what is returned with and without decimal

@muryno its wrong, '.' doesnt have to be always '.', it matters on your languague

this is used for it "DecimalFormatSymbols.getInstance().decimalSeparator.toString()" and in main activity there is decimalSeparatorSymbol variable

Darkempire78 commented 1 month ago

But factorial forms as "3.03!" still does not work

muryno commented 1 month ago

But factorial forms as "3.03!" still does not work

I will take a look at this and update the code

muryno commented 1 month ago

@Darkempire78 The issue was due to precision loss from converting BigDecimal to double for mathematical operations, which can introduce inaccuracies in calculations, especially for functions like the Lanczos approximation that are sensitive to precision.

I have updated private fun gammaLanczos(x: BigDecimal): BigDecimal {logic to minimizes precision loss by managing conversions between BigDecimal and double

Darkempire78 commented 1 month ago

Thank you!

Darkempire78 commented 1 month ago

@muryno I wasn't paying attention, but the pull request has created a problem. It no longer passes the calculation tests. In fact (3!)! no longer works. image

muryno commented 1 month ago

@muryno I wasn't paying attention, but the pull request has created a problem. It no longer passes the calculation tests. In fact (3!)! no longer works. image

@Darkempire78 will take a look at this today ..... or we can create a bug ticket and assign to me

muryno commented 1 month ago

@Darkempire78 MR fix https://github.com/Darkempire78/OpenCalc/pull/440