Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PEMDAS issues #6

Open
adnan360 opened this issue Mar 25, 2021 · 20 comments
Open

PEMDAS issues #6

adnan360 opened this issue Mar 25, 2021 · 20 comments

Comments

@adnan360
Copy link

First of all thanks for implementing PEMDAS. 🏆 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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

adnan360 commented Mar 25, 2021

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

@Phate6660
Copy link
Owner

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
Copy link
Owner

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
Copy link
Owner

It's pushed!

@Phate6660
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

adnan360 commented Mar 28, 2021

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
Copy link
Owner

Phate6660 commented Mar 28, 2021

./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
Copy link
Owner

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

@Phate6660
Copy link
Owner

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
Copy link
Author

./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
Copy link
Author

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
Copy link
Author

I think I got another issue:

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

Answer should be 6 and 8

@Phate6660
Copy link
Owner

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 adnan360 mentioned this issue Mar 29, 2021
@adnan360
Copy link
Author

No problem. Take your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants