logical operators for option type #8369

krux02 · 2018-07-19T19:51:45Z

No description provided.

krux02 · 2018-07-19T20:30:33Z

This is a direct competitive to the optAnd and optOr macros in this PR

kaushalmodi · 2018-07-19T21:27:54Z

lib/pure/options.nim

@@ -312,3 +340,77 @@ when isMainModule:

      let noperson = none(Person)
      check($noperson == "None[Person]")
+
+    test "logical operatior()":


cosmetic typo: operatior -> operator

true. But it's a test case. I don't think it matters.

That's why I said "cosmetic" :) But I personally fix those too.

It's not a huge deal but it certainly matters

mratsim

Code style: I prefer if we use the least powerful construct (inline proc) instead of template if we can.

or and and are better than optOr and optAnd, but due to potential confusion with Option[bool], I would like a different name for option combinators. Rust uses and_then and or_else for example.

Tagging @PMunch

mratsim · 2018-07-20T08:17:04Z

lib/pure/options.nim

@@ -139,6 +139,34 @@ proc get*[T](self: Option[T], otherwise: T): T =
  else:
    otherwise

+template `or`*[T](x: Option[T]; y: T): T =


If an inline proc is enough, I much prefer them than templates

mratsim · 2018-07-20T08:17:30Z

lib/pure/options.nim

+  get(x,y)
+
+template `or`*[T](x, y: Option[T]): Option[T] =
+  let xx = x


inline proc wouldn't need this for example

Would an inline proc guarantee that y is not run if x has a value though? AFAIK it still runs procedures to generate an Option to pass in.

@mratsim an linline proc evaluates all arguments before entering the function body. I explicity want to avoid that. The logical or operator also works like that. That is why you can write conditions like this: if a != nil and a.member > 123. If or would evaluate all arguments first, this would crash when a is nil.

Right, that's a good reason :). I would add a comment using templates to allow evaluation shortcuting so that people don't change it to proc later then.

@mratsim I have a testcase that covers that.

Varriount · 2018-07-20T10:17:29Z

What's the difference between this and #8358 ?

Or rather, what does this do that #8358 doesn't?

PMunch · 2018-07-20T10:30:45Z

Nothing at the moment. When this was created there was a difference, but I copied this solution into mine when I rewrote it.

dom96 · 2018-07-20T10:42:31Z

I'm okay with the or but the and I just find weird and unintuitive.

Bulat-Ziganshin · 2018-07-20T17:32:08Z

I think that changes to "and/or" require discussion as RFC. In particular, I propose the following, more generic approach that I used to deal with empty arrays too:

template `|||`(a,b:T):T = if isTrue(a): a; else: b
template `&&&`(a,b:T):T = if isTrue(a): b; else: a
template `&&&`(a:T,b:U):U = if isTrue(a): b; else: Null(b)
template iif(a:T; b,c:U):U = if isTrue(a): b; else: c

where isTrue and Null are overloaded for each type (bool, Option[T], seq[T]...)

krux02 · 2018-07-20T19:31:53Z

@Bulat-Ziganshin I don't like the idea to introduce new operators and even a new kind of iff at all. I don't like to think "Hmm, which kind of or operator do I need to use in this context", I would rather like "can I use the default or also in this context, let's try it. Ah yea I can and it just works. Also the iff is also known in math as the if and only if, whis is clearly not what you meant.

My original idea to overload these operators for options comes from elisp (emacs lisp). elisp has a builtin called and, and a builtin called or. Both take an arbitrary amount of arguments and an arbitrary amount of arguments and do boolean logic. It is just the case that elisp doesn't have a boolean type. Everything that is not nil is true. So the or-operation evaluates to the last expression in the argument list. That turned out to be very useful in many case. But that is not the only langage that has this feature. Also shell scripting allows to join several commands with an && or an ||. The && is very useful in compiling, for example gcc main.c && ./a.out will only execute the fresh executable, when compilation was succesfull. Or you can exen do this gcc main.c && echo success || echo fail. I would like to see these featuers of chaining commands together in Nim, to. I also try to apply everything that I learned in type theory and boolean logic and just plain programming practice. So the or operator on an option and a non-option will always return a value, similar to x ∨ true will always be true. So there is no point for this operation to return an Option type. I know the limitations of this approach. I can't go inifite and overload or for everything. I can't interpret x or y with x and y of non-Option type T like it was some(x) or some(y) and evaluate it to some(x), because x or y with x and y of type int already has a meaning and that is the bitwise or operation.

krux02 · 2018-07-20T19:43:24Z

@PMunch there are still lots of differences. Just look closely at the tests that I asked you to adopt. You changed a lot of it in your version and added much more verbosity.

PMunch · 2018-07-21T00:14:01Z

Lots of differences is an over-statement. I copied your or and and implementation (although I've yet to add the ones that take one option and one other type after I removed the converters). The tests are also almost exactly the same after the latest rewrite, although I use my comparators as well.

krux02 · 2018-07-22T16:47:51Z

@dom96 I removed the and operator. You showed me that it causes confusion and the overall value is very little. I only added because of "completeness" anyway. I don't think there is real need for it. I also added an example and documentation. I hope you like it.

Araq · 2018-07-23T20:13:38Z

lib/pure/options.nim

+## With the ``or`` operator you can chain together several expressions
+## of type ``Option[T]`` and the whole expression will evaluate the
+## operand from left to right until an the operand that is not
+## none.  This operand will be returned.  If all operands are none, then


We are not British, we use one space after a dot. (nitpick, I know)

dom96

The code is fine, the docs could be better.

dom96 · 2018-07-23T20:16:29Z

lib/pure/options.nim

+## of such an expression does not need to be of type ``Option[T]``, it
+## can be just of type ``T`` for the last fallback. If that is the
+## case the whole expression will be of type ``T``, because it could
+## not evaluate to none anymore.


Your docs, while appreciated, are far too verbose.

Create a short, and to the point heading:

## Retrieving values from an option ## ======================== ## ## Values can be retrieved in many ways, primarily using ``get`` or the ``or`` operator. ## ## Using ``get`` ## -------------- ## ## This is the simplest approach: ## ## ..code-block:: .... ## ## ## But it means there is an exception raised when the option isn't a ``some``. ## ## Using ``or`` ## ------------- ## ## ....

Short sentences, and short specific examples are always better than a huge paragraph and a huge example.

Varriount · 2018-07-31T02:19:41Z

Hrm, I still think this conflates the wrapped type's or with an options or - I would prefer an optOr procedure instead.

Bulat-Ziganshin · 2018-07-31T02:36:47Z

@Varriount 'optOr' isn't even close as readable - look at examples.

Overall, I proposed RFC-style discussion what 'or' should mean. I think that we should just make a general decision and then go together in the chosen way.

Araq · 2019-02-06T20:12:15Z

Sorry, rejected. If in doubt, leave it out.

krux02 added 2 commits July 19, 2018 18:44

wip

81138f5

added tests

46e0160

krux02 changed the title ~~logical operatiors for option (testing magichub to create this PR)~~ logical operatiors for option Jul 19, 2018

comment changes

5e07428

kaushalmodi reviewed Jul 19, 2018

View reviewed changes

mratsim reviewed Jul 20, 2018

View reviewed changes

mratsim mentioned this pull request Jul 20, 2018

Add a safe pattern and special forms for options #8358

Closed

krux02 added 2 commits July 22, 2018 18:31

fixes

7c9e9fb

Removed the and operator, because of minor use case.

eeca441

Araq reviewed Jul 23, 2018

View reviewed changes

dom96 requested changes Jul 23, 2018

View reviewed changes

krux02 changed the title ~~logical operatiors for option~~ logical operators for option type Oct 24, 2018

Araq closed this Feb 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

logical operators for option type #8369

logical operators for option type #8369

krux02 commented Jul 19, 2018

krux02 commented Jul 19, 2018 •

edited

Loading

kaushalmodi Jul 19, 2018

krux02 Jul 19, 2018

kaushalmodi Jul 19, 2018

dom96 Jul 20, 2018

mratsim left a comment

mratsim Jul 20, 2018

mratsim Jul 20, 2018

PMunch Jul 20, 2018

krux02 Jul 20, 2018

mratsim Jul 21, 2018

krux02 Jul 22, 2018

Varriount commented Jul 20, 2018 •

edited

Loading

PMunch commented Jul 20, 2018

dom96 commented Jul 20, 2018

Bulat-Ziganshin commented Jul 20, 2018

krux02 commented Jul 20, 2018

krux02 commented Jul 20, 2018

PMunch commented Jul 21, 2018 •

edited

Loading

krux02 commented Jul 22, 2018

Araq Jul 23, 2018

dom96 left a comment

dom96 Jul 23, 2018

dom96 Jul 23, 2018

Varriount commented Jul 31, 2018

Bulat-Ziganshin commented Jul 31, 2018

Araq commented Feb 6, 2019

logical operators for option type #8369

logical operators for option type #8369

Conversation

krux02 commented Jul 19, 2018

krux02 commented Jul 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mratsim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Varriount commented Jul 20, 2018 • edited Loading

PMunch commented Jul 20, 2018

dom96 commented Jul 20, 2018

Bulat-Ziganshin commented Jul 20, 2018

krux02 commented Jul 20, 2018

krux02 commented Jul 20, 2018

PMunch commented Jul 21, 2018 • edited Loading

krux02 commented Jul 22, 2018

Choose a reason for hiding this comment

dom96 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Varriount commented Jul 31, 2018

Bulat-Ziganshin commented Jul 31, 2018

Araq commented Feb 6, 2019

krux02 commented Jul 19, 2018 •

edited

Loading

Varriount commented Jul 20, 2018 •

edited

Loading

PMunch commented Jul 21, 2018 •

edited

Loading