Skip to content

Add logical operators to the library#53

Merged
notKamui merged 4 commits into
notKamui:devfrom
EgorBron:logical-ops
Apr 18, 2025
Merged

Add logical operators to the library#53
notKamui merged 4 commits into
notKamui:devfrom
EgorBron:logical-ops

Conversation

@EgorBron
Copy link
Copy Markdown
Contributor

Hey there,
I’ve been playing around with your library—really cool work! While testing it out, I noticed there’s no built-in support for logical operations like NOT, AND, OR, EQUALS, and so on. I figured these might be useful in some cases, so I put together a PR that adds basic support for them as functions.

Would love to hear your thoughts—do you think this is something that fits the library’s scope? I'm also happy to adjust or improve anything you'd like.

@notKamui
Copy link
Copy Markdown
Owner

That is indeed within the scope of this library.

In fact, you could make those as part of the DEFAULT_OPERATORS (don't forget to define ! with KevalBothOperator, if kept at all)

Although, I'm not too sure about them being operators, I would prefer them to only be functions. Currently, as you know, there is a limitation in operator names: they can only be one character (i kind of regret this, and now that i changed the grammar, it should theoretically be possible to have operators with more than one character), anyway; this leads to inconsistent operators like = not having its counterpart !=.

Also, because they do kind of go outside of algebraic maths, functions seem better suited to me. If the user still wants operators, its not too hard to add by themselves.

Final reason is linked to #54 : ambiguity with bindings with =

Copy link
Copy Markdown
Owner

@notKamui notKamui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • make the logical set part of the default set of resources
  • only use functions

@EgorBron
Copy link
Copy Markdown
Contributor Author

Thanks for your review! Now I think that using only functions is clearer and better than defining corresponding operators, even if we would to define them in way with multiple characters.

But I have one question before I will fix the things you highlighted: should we limit ourselves to using short names for functions (e.g., eq, gt) or should we also adopt longer, more descriptive names—or perhaps both?

@notKamui
Copy link
Copy Markdown
Owner

But I have one question before I will fix the things you highlighted: should we limit ourselves to using short names for functions (e.g., eq, gt) or should we also adopt longer, more descriptive names—or perhaps both?

Hi ! I would generally advise against abbreviations, but I feel in this case they're so within well-known standards that it is acceptable. As whether to keep both or not, I think only the abbreviations will suffice.

so

eq, ne, gt, ge, lt, le, not, or, and, nor, xor, and finally, xnor and nand for completeness

@EgorBron
Copy link
Copy Markdown
Contributor Author

EgorBron commented Apr 17, 2025

As whether to keep both or not, I think only the abbreviations will suffice.

Got you, I’ll keep only the abbreviated names.

and finally, xnor and nand for completeness

Ah, right! Will add them too, with also implication (IMPLY) and nonimplication (NIMPLY).

Note just for myself: logical operations on regular numbers is an interesting case in here. E.g., the XNOR operation, is also known as *logical equality*, which evaluates to "either both numbers are zeros or not equal to zero".

@EgorBron
Copy link
Copy Markdown
Contributor Author

EgorBron commented Apr 17, 2025

I’m also gonna make all of these functions variadic (except comparisons for greater/lower and for implications), just in case. If the resulting overhead of processing DoubleArrays to compute results of operations is not what we need, let me know!

@EgorBron EgorBron requested a review from notKamui April 17, 2025 14:24
@EgorBron
Copy link
Copy Markdown
Contributor Author

EgorBron commented Apr 17, 2025

Everything seems done, now awaiting for your review!

P.S. I’ll leave the README editing to you.

Copy link
Copy Markdown
Owner

@notKamui notKamui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR, thank you very much for your time and help ! Will publish ASAP (I hope Maven Central didnt change too much since the last publish 🥲)

@notKamui notKamui merged commit 599e36c into notKamui:dev Apr 18, 2025
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 this pull request may close these issues.

2 participants