On 2014-15-07 21:41, Andy Parker wrote:
On Tue, Jul 15, 2014 at 11:01 AM, rahul <[email protected]
<mailto:[email protected]>> wrote:
[..snip..]
and then there's been some discussion on the PR around these
two cops:
Style/AndOr
Lint/AssignmentInCondition
Each of those two checks catch coding patterns which both
are a source of some bugs and, at the same time are
idiomatic in certain cases. So there's room for discussion
on those two.
The only idiomatic part of the and/or one that we'd get rid of
is failure statements: do_something() or raise "Noooo!!!". I'm
willing to drop that for the slightly more wordy:
if !do_something()
raise "Nooooo!!!!"
end
In fact, I often like that construction better because it is
more future proof. If it turns out that you need to add extra
actions in the error case, then there isn't any need to perform
refactor first.
It is possible to get back most idiomatic constructions by slightly
tweaking the cops. In fact it is rather easy to do so, and here[1]
is a custom cop that checks for And/Or only in conditionals.
- I feel that the boolean control flow idiom is actually useful.
A pattern that I have seen often in code is of the type
unless x = mymethod()
x.do_something
end
I think you unwittingly just provided an example of exactly why I like
neither assignments in expressions nor unless. What you just wrote is,
"call do_something on x if x is nil or false". It is really easy to
write that unintentionally with assignments and unless combined. :)
Which falls foul of Lint/AssignmentInCondition
I think that it would be better written as
`x = mymethod() and x.do_something`
rather than
`(x = mymethod()) && x.do_something`
which is recommended in the ruby style guide from rubocop if we
restrict and/or to purely control flow operators.
This is starting to get into territory of religious wars, but I think we
should be looking for what construction is the most obviously correct
from the standpoint of a reader. I find that "and" isn't the most
straightforward nor easily changed construction. For instance, your
example could be written without "and" as:
I do not like the "and" "or"; to me code is clearer without them.
x = mymethod()
if x
x.do_something()
end
Now, granted, that is 4 lines instead of 1, but it also makes a few
things really obvious to me:
You now also have 1 assignment, and two references to x. Unfortunately
we have to care very much about manual optimization. If that was not the
case I would agree. I think use of and/or does not have any performance
advantages, they are basically conditional test/jumps.
- henrik
--
Visit my Blog "Puppet on the Edge"
http://puppet-on-the-edge.blogspot.se/
--
You received this message because you are subscribed to the Google Groups "Puppet
Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/puppet-dev/lq43hv%24ge0%241%40ger.gmane.org.
For more options, visit https://groups.google.com/d/optout.
