Created attachment 689456 [details] screenshot #1 For example, the number 1.4322661567719377e+25 (you can get a big number by multiplying big numbers) (screenshot #1), multiplied by 9 (screenshot #2), gives 226.432 (screenshot #3), which is totally wrong.
5 years ago
blocking-basecamp: --- → ?
This app isn't a Calculator for scientists
blocking-basecamp: ? → -
And the result is correct.
What it's doing is 25*9 then adding +1.432 the result is correct in that e is not being counted.
Oh! My mistake. I see what you mean by getting the large number. I thought the e was being typed. The value after getting an e is incorrect... This is kinda bad, we also end up rounding up at the 3 decimal place. clee, if you think this is bad please renom.
(In reply to dscravaglieri from comment #4) > And the result is correct. the result should be wrong. 4.320e+3 is equal to 4320. in this case, the result will be 1.4322661567719377e+25 (due to the number of 9 is too small to the number, it will be ignored)
Created attachment 691870 [details] [diff] [review] patch Here I propose a better parser for numbers and operators. Give me a feedback :)
Created attachment 692079 [details] [diff] [review] patch b I just moved my code in a separated file. It's easier to debug it.
Hey, I think this is definitely important to support. I was actually working on another bug, and had to fix this for the end solution. My open pull request is located here: https://github.com/mozilla-b2g/gaia/pull/6934 Essentially I'm just converting the numbers to a string before the evaluation, whereas you are tokenizing. The end solution doesn't really matter to me, I do really just want to see this fixed.
(In reply to Al Tsai [:atsai] from comment #7) > (In reply to dscravaglieri from comment #4) > > And the result is correct. > > the result should be wrong. > > 4.320e+3 is equal to 4320. > in this case, the result will be 1.4322661567719377e+25 (due to the number > of 9 is too small to the number, it will be ignored) Damn it. I found that is a multiply calculation, and the result should be something like 1.2890394...e+26 Current result is definitely a wrong one.
> My open pull request is located here: > https://github.com/mozilla-b2g/gaia/pull/6934 I think we should have this code reviewed before. Take a look of my patch, maybe we can merge the 2 approaches. I implemented a 'full' math expression parser, a float number parser etc.
Calculator is now a v1 core app so we need this fixed. Let's mark blocking. (We can't have the results of the calculator wrong, that's not an option.)
blocking-basecamp: - → ?
Attachment #692079 - Flags: feedback?(dale) → review?(dale)
Comment on attachment 692079 [details] [diff] [review] patch b Very sorry for dropping the ball in this review, I thought I was up to date until I checked 'My Requests' today The implementation of the number parser is cleaner in my opinion, however this fix will not fix the precision of the result bug that kevin's patch does. Andrea would you mind integrating Kevins tests into your patch (which will require the formatNumber fix as well) I will clear the review request until we have the extra tests in place, however the code looks good and is working good for me here
Attachment #692079 - Flags: review?(dale)
Created attachment 693423 [details] [diff] [review] patch c Tested added.
Renom - bug 822565 indicates calculator app is no longer in v1.
blocking-basecamp: + → ?
5 years ago
Attachment #693423 - Flags: review?(dale) → review+
Ah, sorry this got renomed right in the middle of me reviewing + merging Merged in: https://github.com/daleharvey/gaia/commit/77a7a987566a6834307b981ce78c15943c4f1615
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Calculator app is not part of Gaia anymore.
Status: RESOLVED → VERIFIED
Resolution: FIXED → INVALID
For v1. It's still a bug for future that we should check if it does get readded. As far as I understand we are adding it back in. removing it from invalid.
Status: VERIFIED → RESOLVED
Last Resolved: 5 years ago → 5 years ago
Resolution: INVALID → FIXED
Component: Gaia::Calculator → Gaia::Calculator
Product: Boot2Gecko → Boot2Gecko Graveyard
You need to log in before you can comment on or make changes to this bug.