search

Phate6660 / bcalc

A pure-bash calculator in which I even wrote my own lexer and parser for.
Other
5 stars 1 forks source link

PEMDAS issues #6

Open adnan360 opened 3 years ago

adnan360 commented 3 years ago

First of all thanks for implementing PEMDAS. :trophy: It was great to see you implement it that fast.

I did some testing:

$ ./bcalc '(2+2)*2'
2
$ ./bcalc '2*(2+2)'
26
$ ./bcalc '2^(2+2)'
26
$ ./bcalc '(2+2)^2'
3
$ ./bcalc '(2)^2'
1
$ ./bcalc '2+2+2^2'
8

All of these are wrong. Am I doing anything wrong?

adnan360 commented 3 years ago

I've tried with adding [...] to the numbers and the results were correct. But readme says "[] = denotes a multi-digit number". 2 is not multi-digit, so it should not require square brackets if I understand correcly.

Phate6660 commented 3 years ago

Hm, this is probably an issue with the parser. Oh wait, actually think I see why. It'll have to wait though, I have to be away from PC for a while.

adnan360 commented 3 years ago

No problem. Good things take time. :)

But I'd be happy if I didn't have to put in all the [] for multidigit numbers too in the future. It takes time to type them for each number.

Phate6660 commented 3 years ago

Oh don't worry, that'll only be temorary. I'm working on just allowing multi-digit characters like normal, I just forgot to stick it in the readme.

adnan360 commented 3 years ago

That'd be great. Take your time though. No hurry from my side.

Phate6660 commented 3 years ago

Hey, I just wanted to let you know that I've started to work on it. I re-wrote a bit of the parser. It's fairly error prone (which is why it's a WIP on a separate branch), but you can at least now use multiple-digit numbers without specifying them with []. You can track the progress here: https://github.com/Phate6660/bcalc/tree/multi-digit-and-parser

Phate6660 commented 3 years ago

I believe I got it working! I just need to tidy up the script a bit, add some finishing touches to the README. Then I'll push it the branch. When I do, would you mind testing it again? Before I merge it into master.

Phate6660 commented 3 years ago

It's pushed!

Phate6660 commented 3 years ago

I just realized I forgot to actually explain what I did. I believe now I got it to actually support PEMDAS, I tested it myself. And [] is now no longer needed. You can you multiple-digit numbers just like normal now.

Phate6660 commented 3 years ago

I actually feel pretty confident about this. All of the calculations I've tried have worked. Obviously I'm not going to try every combination, so there's bound to be more bugs. But I feel like this is a stable and working enough base to merge. I'm going to keep this issue open, feel free report anything you find here! :)

adnan360 commented 3 years ago

WOW. Thanks for implementing it that fast.

With my initial checks it seems to be working fine. I think it's ready to be included in my config. I'll close it if I find no more issues.

adnan360 commented 3 years ago

I think I found some issues with exponents:

$ ./bcalc '(2^2)*2'
0
$ ./bcalc '(2^2)*1'
0
$ ./bcalc '9^(1/2)'
1

Tested on master

Phate6660 commented 3 years ago

./bcalc '9^(1/2)' actually isn't wrong.

1/2 = 0 If you want the remainder, you need to use %. 1%2 = 1

9^0 = 0 9^1 = 9

I'll have to look into the other ones though.

Phate6660 commented 3 years ago

That is my bad though, I forgot to document that % is a valid math operation.

Phate6660 commented 3 years ago

I fixed the other two. The problem was that I forgot to convert ^ to ** while calculating things in parentheses. As ** is what bash uses for exponents.

adnan360 commented 3 years ago

./bcalc '9^(1/2)' actually isn't wrong.

Well, I have been taught in school that having an exponent of 1/2(=0.5) is like having a sqroot. I've run this through multiple online calculators and they all seem to output 3. (1) (2)

But this is in a way my fault too. 1/2 returns decimal, so it should not work anyway until decimals are implemented. So I wouldn't be worried about it too much now. But a message would be nice when decimals/fractions like this occur.

adnan360 commented 3 years ago

I fixed the other two. The problem was that I forgot to convert ^ to while calculating things in parentheses. As is what bash uses for exponents.

Yeah, thanks. Tested with some basic inputs and seems to be working fine now.

adnan360 commented 3 years ago

I think I got another issue:

$ ./bcalc '((2+2)+2)'
4
$ ./bcalc '(((2+2)+2)+2)'
224

Answer should be 6 and 8

Phate6660 commented 3 years ago

Yeah, I can already see why. The parser was not built with nested parenthesis in mind. I'll definitely need to re-work the parser to account for that. Or at least the parenthesis part anyway.

Sorry for the late reply, I'm away from PC. Spending some time with the family right now. I just had a little time to comment because we're taking a slight break to make food.

adnan360 commented 3 years ago

No problem. Take your time.