Skip to content
Open
Changes from all commits
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
74 changes: 73 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,79 @@ _You do not need to commit the generated `./doc` or `.yardoc` files._
end
```

- The `and` and `or` keywords are banned. It's just not worth it. Always use `&&` and `||` instead.
- Do not use `and` and `or` in boolean context - `and` and `or` are control flow operators and should be used as such. 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.

```ruby
# good: and/or for control flow
x = extract_arguments or raise ArgumentError, "Not enough arguments!"
user.suspended? and return :denied
Copy link

@fran-fac fran-fac Jan 27, 2023

Choose a reason for hiding this comment

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

Would we want to use...

user.suspended? and return :denied

instead of a guard if?

return :denied if user.suspended?

How do we choose? A bit wary of opening up a "two ways to do one thing" situation, when the existing codebase is consistently using a form that looks just as powerful.

The feeling I have is that and/or are more readable when the method call reads like an action:

do_something or return :failure

while guard if/unless work better with boolean methods:

return :invalid unless is_valid?(param)

However, the first case requires the method to return a truth-y value, which feels like... I suppose it reminds me of PHP, where most methods return false|value and and/or are often used for control flow – but there are not guard ifs in PHP!

Copy link
Author

Choose a reason for hiding this comment

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

I would go with the return...if if that case. I do think think this is a poor example when you could use a guard if instead.

As you say, I think and/or work better when you're checking whether an action was successful. You can achieve the same thing with if/unless but I find that is actually less readable in the action case:

do_something or return :failure

return :failure unless do_something

Happy to modify the examples if you think that pattern makes sense.

Copy link

Choose a reason for hiding this comment

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

In our examples above, do_something needs to return a truth-y value, but does not guarantee a boolean as do_something? would.

Are we putting a invisible dependency on the called method then, and risk someone changing do_something in a perfectly valid way, which breaks the implicit truth-y-ness return requirement? Is that an actual risk, and is it acceptable in respect to what we gain from relaxing the rules around and/or?

(Maybe the above is not very clear, I'll try again with a fresh brain tomorrow)


# 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
# &&/|| in conditions
if got_needed_arguments && arguments_valid
# ...body omitted
end
# in logical expression calculation
ok = got_needed_arguments && arguments_valid

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

Avoid several control flow operators in one expression, as that quickly
becomes confusing:

```ruby
# 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

# 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
```

NOTE: Whether organizing control flow with `and` and `or` is a good idea has been a controversial topic in the community for a long time. But if you do, prefer these operators over `&&`/`||`. As the different operators are meant to have different semantics that makes it easier to reason whether you're dealing with a logical expression (that will get reduced to a boolean value) or with flow of control.

Why is using `and` and `or` as logical operators a bad idea?

Simply put - because they add some cognitive overhead, as they don't behave like similarly named logical 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.

```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
```

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.

```ruby
true or true and false # => false (it's effectively (true or true) and false)
true || true && false # => true (it's effectively true || (true && false)
false or true and false # => false (it's effectively (false or true) and false)
false || true && false # => false (it's effectively false || (true && false))
```

- Avoid multi-line `?:` (the ternary operator), use `if/unless` instead.

Expand Down