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

Comparisons and logical operators #29

Open
cschreib opened this issue Jun 30, 2017 · 10 comments
Open

Comparisons and logical operators #29

cschreib opened this issue Jun 30, 2017 · 10 comments

Comments

@cschreib
Copy link

cschreib commented Jun 30, 2017

I've been using TinyExpr in my project and recently needed standard comparison operators (<, <=, >, >=) and boolean logic (!, &&, ||). I've just implemented this in my fork with this commit cschreib@c1b01d8 and this one cschreib@4cdeb91 (adding == and !=).

Booleans are simply 0.0 for false and 1.0 (or any non-zero value) for true.

I've written a test suite to make sure that the precedence with respect to other binary operators is correct, and it all seems to work fine. There is one little thing that I do not understand though, is that I was expecting !!1 to produce 1, since --1 produces 1 in TinyExpr. I do not understand what the difference is, but !!1 currently produces 0 regardless of how many negation operators are used. It's no big deal for me, but it would be good if this worked to be consistent with the other unary operators.

Let me know if you're interested in merging this feature, then I'll prepare a pull request.

@cschreib
Copy link
Author

cschreib commented Jul 1, 2017

Commit cschreib@9de3ed2 fixes the issue with !!1 != 1. Thanks to @codeplea for spotting the mistake.

This commit also fixes the right-to-left version of TinyExpr (which I had broken badly by mistake), and will correctly optimize stuff like -!--!-a into -!!a (note: !! is not a no-op for numbers which are neither 0 nor 1: !!2 == 1).

@godlikepanos
Copy link

This seems like a useful feature. Have you considered making a pull request?

@cschreib
Copy link
Author

cschreib commented Jun 4, 2018

I have considered it, but as per the guidelines set in https://github.com/codeplea/tinyexpr/blob/master/CONTRIBUTING I have opened an issue first to gauge interest. I interpret the subsequent lack of reply as a hint that this is not a desired feature, but I'm happy to be wrong :)

Also, my fork was based on another fork which included CMake build scripts. Adding these CMake scripts has been proposed here and rejected, so perhaps this is also a source of restraint.

@codeplea
Copy link
Owner

Sorry for the very late reply.

I will likely merge this into a branch, spend some time tweaking and adding documentation, and then merge into master. I've already created a pull request here.

Most of my hesitation was exactly as you expected. However, I've had people ask for both CMake support and logical operators recently, so I'm giving in. It only took a year. lol

Nice work!

@cschreib
Copy link
Author

Excellent news! Glad you found it to be an interesting addition.

@tanis2000
Copy link

It would be nice to see this branch get merged into master now. Are there any plans?

@codeplea
Copy link
Owner

codeplea commented Mar 1, 2019

It's merged into the "logic" branch. It needs documentation before going to master. It also needs test cases added.

I won't have time to work on it for a while. Feel free to help.

@gfmartins
Copy link

gfmartins commented Aug 26, 2019

If I'm not wrong, the Operator Precedence, in the logical branch is wrong.
In the branch the precedence is this:
[1] > | < | >= | <= | == | !=
[2] AND | OR

it should be:
[1] > | < | >= | <=
[2] == | !=
[3] AND
[4] OR

https://en.cppreference.com/w/c/language/operator_precedence

@Blake-Madden
Copy link
Contributor

I forked a C++ version of this library that includes logic and comparison operators in case that is useful to anyone:

https://github.com/Blake-Madden/tinyexpr-plusplus

@kdunn926
Copy link

kdunn926 commented Sep 8, 2022

It's merged into the "logic" branch. It needs documentation before going to master. It also needs test cases added.

@codeplea - There seem to be a reasonable number of test cases, were there others you had in mind for rounding out what is already there?

I'll work on adding some documentation.

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

7 participants