Skip to content

Unban and and or control flow operators #730

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

Merged
merged 2 commits into from
Jul 26, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 41 additions & 35 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1420,57 +1420,63 @@ 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 should only be used
as a final fallback statements.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why fallback? I guess you meant final expression in a method in general, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bbatsov I tried to adjust the wording, making it more explanatory. I also moved the "good usage" example to be the first one, so it also works as a demo of the concept.


[source,ruby]
----
# bad
# boolean expression
ok = got_needed_arguments and arguments_are_valid

# control flow
document.save or raise("Failed to save document!")
# and/or in conditions
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
# &&/|| in conditions
if got_needed_arguments && arguments_valid
# ...body omitted
end
# in logical expression calculation
ok = got_needed_arguments && arguments_valid

# control flow
raise("Failed to save document!") unless document.save
# bad
# &&/|| for control flow (can lead to very surprising results)
x = extract_arguments || raise(ArgumentError, "Not enough arguments!")

# ok
# control flow
document.save || raise("Failed to save document!")
# good: and/or for control flow
x = extract_arguments or raise ArgumentError, "Not enough arguments!"
user.suspended? and return :denied
----

.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 community for a long time. But if you do, use these operators and not `&&`/`||`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

whether -> Whether
use these -> prefer these operators over


=== Multi-line Ternary Operator [[no-multiline-ternary]]

Expand Down