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

incorrect evaluation of powers #29

Open
boxxxie opened this issue Mar 19, 2018 · 2 comments · May be fixed by #46
Open

incorrect evaluation of powers #29

boxxxie opened this issue Mar 19, 2018 · 2 comments · May be fixed by #46

Comments

@boxxxie
Copy link

boxxxie commented Mar 19, 2018

2^2^2^2 will give a result that is like (((2 ^ 2) ^ 2) ^ 2) which is 16^2

i actually have my own implementation of what you wrote, that relies on clojure.walk

if you are ok with that, i can issue a PR with some more tests

@rm-hull
Copy link
Owner

rm-hull commented Mar 19, 2018

Is this the infix macro or the from-string (or both?)

would be interested to see the clojure.walk version + how well of a replacement it would be if it is cleaner & provides the same or better functionality

@boxxxie
Copy link
Author

boxxxie commented Mar 20, 2018

clojure walk version is built off of what you already have, just doesn't have the notion of recursing
it's a bit messy for doing the "^" conversion (lots of reversing, but this is code that gets called almost never).

i'll link a gist of the code i use

https://gist.github.com/boxxxie/8a988b4fc5f35d31eb2d4f6f35799463

so, main difference is that i have a mapping of which functions need to be rewritten forward, then backward.
i do 2 stages of rewrite, an infix->paren rewrite, then infix->prefix

my functions only deal with binary operators, while your code handles unary. i think it would be easy for me to add that functionality back in (i just had different requirements for the code).

the rewrite-op-backwards rewrite-op-forwards have duplicate code, so they can be refactored

@emlyn emlyn linked a pull request Sep 27, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

2 participants