Skip to content

Commit

Permalink
Unban and and or control flow operators (#730)
Browse files Browse the repository at this point in the history
  • Loading branch information
zverok authored Jul 26, 2022
1 parent f04159a commit bb8cee1
Showing 1 changed file with 44 additions and 36 deletions.
80 changes: 44 additions & 36 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1420,57 +1420,65 @@ def banned?
end
----

=== `and`/`or` [[no-and-or-or]]
=== `and`/`or` [[no-and-or-or]] [[and-or-flow]]

The `and` and `or` keywords are banned.
The minimal added readability is just not worth the high probability of introducing subtle bugs.
For boolean expressions, always use `&&` and `||` instead.
For flow control, use `if` and `unless`; `&&` and `||` are also acceptable but less clear.
`and` and `or` are control flow operators. They have very low precedence, and can be used as a short
form of specifying flow sequences like "evaluate expression 1, and only if it is not successful
(returned `nil`), evaluate expression 2". This is especially useful for raising errors or early
return without breaking the reading flow.

[source,ruby]
----
# bad
# boolean expression
ok = got_needed_arguments and arguments_are_valid
# good: and/or for control flow
x = extract_arguments or raise ArgumentError, "Not enough arguments!"
user.suspended? and return :denied
# control flow
document.save or raise("Failed to save document!")
# bad
# and/or in conditions (their precedence is low, might produce unexpected result)
if got_needed_arguments and arguments_valid
# ...body omitted
end
# in logical expression calculation
ok = got_needed_arguments and arguments_valid
# good
# boolean expression
ok = got_needed_arguments && arguments_are_valid
# control flow
raise("Failed to save document!") unless document.save
# &&/|| in conditions
if got_needed_arguments && arguments_valid
# ...body omitted
end
# in logical expression calculation
ok = got_needed_arguments && arguments_valid
# ok
# control flow
document.save || raise("Failed to save document!")
# bad
# &&/|| for control flow (can lead to very surprising results)
x = extract_arguments || raise(ArgumentError, "Not enough arguments!")
----

.Why Ban `and` and `or`?
****
The main reason is very simple - they add a lot of cognitive overhead, as they don't behave like similarly named operators in other languages.
First of all, `and` and `or` operators have lower precedence than the `=` operator, whereas the `&&` and `||` operators have higher precedence than the `=` operator, based on order of operations.
But avoid several control flow operators in one expression, that becomes
confusing:

[source,ruby]
----
foo = true and false # results in foo being equal to true. Equivalent to ( foo = true ) and false
bar = false or true # results in bar being equal to false. Equivalent to ( bar = false ) or true
----
# bad
# Did author mean conditional return because `#log` could result in `nil`?
# ...or was it just to have a smart one-liner?
x = extract_arguments and log("extracted") and return
Also `&&` has higher precedence than `||`, where as `and` and `or` have the same one. Funny enough, even though `and` and `or`
were inspired by Perl, they don't have different precedence in Perl.
# good
# If the intention was conditional return
x = extract_arguments
if x
return if log("extracted")
end
# If the intention was just "log, then return"
x = extract_arguments
if x
log("extracted")
return
end
---
[source,ruby]
----
foo = true or true and false # => false (it's effectively (true or true) and false)
foz = true || true && false # => true (it's effectively true || (true && false)
bar = false or true and false # => false (it's effectively (false or true) and false)
baz = false || true && false # => false (it's effectively false || (true && false))
----
****
NOTE: Whether organizing control flow with `and` and `or` is a good idea was a controversial topic in the community for a long time. But if you do, prefer these operators over `&&`/`||`.
=== Multi-line Ternary Operator [[no-multiline-ternary]]
Expand Down

0 comments on commit bb8cee1

Please sign in to comment.