From: Rein Henrichs <reinh@...>
Date: 2010-06-08T02:00:11+09:00
Subject: Re: Processing (potentially) nil method without using nested if

On 2010-06-07 02:24:26 -0700, Robert Klemme said:

> 2010/6/5 Brian Candler <b.candler@pobox.com>:
>> Ben Vishny wrote:
>>> Ideally I'd like to do:
>>> 
>>> if parent.respond_to? "children" && parent.children.size != 0
>>> � �puts "Has Children"
>>> end
>>> 
>>> But I get an error if parent.children is undefined. Isn't there a way of
>>> stopping if the first condition is false?
>> 
>> Yes, but you're being bitten by the ambiguity of what you've written,
>> which is resolved by precedence rules. I'm not sure exactly how your
>> code was being parsed, but I guess something like
>> 
>> if parent.respond_to?("children" && (parent.children.size != 0))
>> 
>> The rule is, if in doubt, add your own parentheses.
>> 
>> The following all work as you expect:
>> 
>> if (parent.respond_to? "children") && (parent.children.size != 0)
>> �puts "Has Children"
>> end
>> 
>> if parent.respond_to?("children") && parent.children.size != 0
>> �puts "Has Children"
>> end
>> 
>> if parent.respond_to? "children" and parent.children.size != 0
>> �puts "Has Children"
>> end
>> 
>> The last of these works because 'and' has very low precedence, but I'd
>> say it's much better to be explicit with parentheses than clever with
>> your knowledge of precedence rules.
>> 
>> Incidentally, it's more idiomatic to use a symbol rather than a string
>> for respond_to?
>> 
>> if parent.respond_to?(:children) ...
> 
> It's even more idiomatic to invoke a method and deal with the
> exception.  Checking with #respond_to? does not guarantee that the
> method can actually be executed.
> 
> In this case you could do
> 
> puts "Has Children" unless parent.children.empty? rescue nil
> 
> Or for a more targeted catch:
> 
> begin
>   puts "Has Children" unless parent.children.empty?
> rescue NoMethodError
>   # OK, no output
> end
> 
> Kind regards
> 
> robert
> 
> --
> remember.guy do |as, often| as.you_can - without end
> http://blog.rubybestpractices.com/

Robert, you are correct in that "It's even more idiomatic to invoke a 
method and deal with the exception" but rescue nil is a code smell that 
I would rather see avoided. A begin/rescue block also seems unnecessary.

Instead of solving a potential non-problem, I think it would be better 
to ask questions like... Why would this parent object not have a 
children method? Does this imply a design inconsistency? If parent can 
be nil, see the following.

In order to avoid NoMethodError on nil bugs, it's most common to use 
&&, which short-circuits, as such:

parent.children && parent.children.size != 0

I may be assuming too much about your domain, but this seems more concise:

parent.children && !parent.children.empty?

Furthermore, it may be useful to ask: Why would children ever be nil? 
The empty array is a semantically correct way to say "no children". If 
children were initialized to [], you could always ask children.empty? 
and skip the check for nil. For instance:

class Parent
  attr_reader :children
  def initialize(children=[])
    @children = children
  end
end

-- 
Rein Henrichs
http://puppetlabs.com
http://reinh.com