Posted on by

Do, or do not. There is no #try.

One of the ways you know you are working in a good language is that it makes bad habits ugly. To wit:

# params[:foo] might be missing
value = params[:foo].try(:[], :bar)

This is not pretty. It is, as my dad might say, “sick and ugly and it wants to die”.
(Yes, your dad would say that. KAG)

It is also, not coincidentally, completely unnecessary. Ruby is a language in which ugly things are more often than not superfluous.

Before I get to rewriting it, let me talk a little about the underlying problem here. The problem is uncertainty. And #try is not the answer. In fact, I’ll come right out and say it: #try is a code smell. It’s a good thing it’s just in ActiveSupport and not in Ruby proper. #try is a thin layer of faded 70s wallpaper pasted over a glaring violation of the “Tell, Don’t Ask” principle.

There is almost never a good excuse to use #try. The example above would be better written with fetch:

# provide a default value (an empty Hash) for params[:foo]
value = params.fetch(:foo){{}}[:bar]

In some cases, where you have control over the original Hash, it can make more sense to give the Hash a default value other than nil:

params = Hash.new { {} }
# ...
value = params[:foo][:bar]

In other cases, when you have a really deeply nested structure, a Null Object might be more appropriate:

# See Null Objects article for implementation of Maybe()
value = Maybe(params)[:foo][:bar][:baz][:buz]

But maybe a Null Object is a bigger gun than you want to haul out for this. I got
to thinking about this problem today, and here’s what I came up with, with some help from Larry Marburger:

h = {
  :foo => {
    :bar => [:x, :y, :z],
    :baz => Hash.new{ "missing value in :bar" }
  }
}
h.extend(DeepFetch)
h[:foo]                         # => {:baz=>{}, :bar=>[:x, :y, :z]}
h[:foo, :bar]                   # => [:x, :y, :z]
h[:foo, :bar, 1]                # => :y
h[:foo, :bar, 5]                # => nil
h[:foo, :baz]                   # => {}
h[:foo, :baz, :fuz]             # => "missing value in :bar"
h.deep_fetch(:foo, :bar, 0)     # => :x
h.deep_fetch(:buz) { :default_value } # => :default_value
h.deep_fetch(:foo, :bar, 5) { :default_value } # => :default_value

Here’s the implementation of DeepFetch (also available as a Gist):

module DeepFetch
  def deep_fetch(*keys, &fetch_default)
    throw_fetch_default = fetch_default && lambda {|key, coll|
      args = [key, coll]
      # only provide extra block args if requested
      args = args.slice(0, fetch_default.arity) if fetch_default.arity >= 0
      # If we need the default, we need to stop processing the loop immediately
      throw :df_value, fetch_default.call(*args)
    }
    catch(:df_value){
      keys.inject(self){|value,key|
        block = throw_fetch_default && lambda{|*args|
          # sneak the current collection in as an extra block arg
          args < < value
          throw_fetch_default.call(*args)
        }
        value.fetch(key, &block)
      }
    }
  end

  # Overload [] to work with multiple keys
  def [](*keys)
    case keys.size
    when 1 then super
    else deep_fetch(*keys){|key, coll| coll[key]}
    end
  end

end

Notable about this implementation:

  • There is no mucking about with #respond_to? or rescue.
  • It’s not just for hashes: this will work with any container that supports #fetch and #[].

I hope someone finds this useful.

EDIT: The discussion of #try and its design ramifications is continued in the next article.

24 comments

  1. For Hashes, I agree.

    For other things like AR classes, I’m not so sure. 

    Consider:

    v.s.

    I prefer the first.. What do you think?

    1. Neither. Why are you violating Demeter? Use:

      human.country_name

      Which can be implemented as:

      class Human
        delegate :name, :to => :country, :prefix => true, :allow_nil => true
      end

      1. I have never been a fan of the “Law” of Demeter. I believe its violations are worth looking at, but I think this is a perfect case of where it falls down.

        Why should a Human have to know whether a Country has a name? Or any other attribute (unless it needs them itself)? If a Human is associated with multiple Countries (birth, residence, voting, vacation, etc.) does it then have to duplicate this delegation for each method of each country?

        What about attributes that clearly have nothing to do with Human? Yes, one might say that a Human has a country_name. But does a Human have a country_population? A country_mortality_rate? I would say it does not, but Demeter insists that it must.

        Of course, the Demeterian way out of this mess is

        country = human.country
        name = country.name
        population = country.population

        but that’s just syntax. It’s still just as much of a Demeter violation in spirit.

        I come from the old school of OOP, which likes lots of little decoupled objects floating around. It doesn’t much care for binding interfaces together like this. A Human has a country. A Country has a name. A Human does not (really) have a country_name.

        To end with reductio ad absurdam, imagine sending an email to the CEO of a company. No company.ceo.email allowed here! Instead, to satisfy this silly “law” you would have to have a company.ceo_email (and ceo_name, and treasurer_street_address_zipcode, and janitor_dental_benefits_beneficiary_responsible_party_first_name, etc., etc.)

        2. IMO, your example misses the mark slightly. If you are sending an e-mail to a company’s CEO, then Law of Demeter says it’s a code smell to do it via company.ceo.email because your method was passed the wrong thing. Instead you might have something like:

              class Mailer
                def deliver!(recipient)
                  send_email_to recipient.email
                end
              end

          Note that messages are delivered to people (e.g. company.ceo), not companies (e.g. company). Now you would write:

              Mailer.new.deliver! company.ceo

          which avoids any Law of Demeter problems.

          1. First, I was responding to the suggestion that delegation is the way to satisfy LoD, and my comments there still stand.

            I would certainly not introduce a whole new class (which has its own dependency issues) to avoid dots. However, I can see that LoD might point out that a Mailer class might be desirable for other reasons.

            But that wouldn’t be correct in all circumstances: http://s3.amazonaws.com/37assets/svn/goji-namecell-68d8407856ac19a13ac20bdb358b5646.jpg

        4. “Of course, the Demeterian way out of this mess is …”

          Being a sometimes Demeterian, I object! Have you actually seen somebody presenting that as non-Demeter? If so, they’re just wrong. Demeter != method chains, but method chains are one obvious way to spot them. Agree that breaking it up into 3 statements is equally a violation.

          As for the company.ceo.email example, that can follow Demeter without adding all those methods. Just pass company.ceo to the code that needs to access its attributes. Now the code sending it does not violate demeter, nor does the code using it. This not only follows Demeter, but it gets its intended benefit: isolation of change. If the Person’s (assuming ceo is a Person) address structure changes, the email-generation code changes, but not the code that passes the ceo to the email.

        5. “Of course, the Demeterian way out of this mess is …”

          Being a sometimes Demeterian, I object! Have you actually seen somebody presenting that as non-Demeter? If so, they’re just wrong. Demeter != method chains, but method chains are one obvious way to spot them. Agree that breaking it up into 3 statements is equally a violation.

          As for the company.ceo.email example, that can follow Demeter without adding all those methods. Just pass company.ceo to the code that needs to access its attributes. Now the code sending it does not violate demeter, nor does the code using it. This not only follows Demeter, but it gets its intended benefit: isolation of change. If the Person’s (assuming ceo is a Person) address structure changes, the email-generation code changes, but not the code that passes the ceo to the email.

          2. Yes, I stand corrected on the “way out of this mess” I proposed. Assigning subobjects to variables that are local is not Demeterian. Assigning subobjects to variables that are method parameters apparently is.

            Even after doing that, though, I still see coupling. By passing the CEO object to a mailer instead of simply accessing its email property, the caller still has the dependency that it knows that a CEO is “emailable.” Further, it has to know that the Mailer object handles emailable objects. If either of those things change in some respect, the caller needs to know about it (for example, if CEO is changed to include multiple email addresses, the caller would need to tell the Mailer which one to use).

            However, I do believe that LoD has merit in detecting smells. But like Martin Fowler, I consider the “law” more of a “suggestion”. For example, in the graphic I posted in response to John, I think it makes sense simply to dig into the company’s CEO object to get his phone number. And I still don’t think that delegation is always the best way to Demeterize code.

  2. Agreed – try is a code smell.

    One project I worked on was riddled with “oh this could be null”, as specified by the customer. If we had used #try we would have gotten… bad code. Unconfident would have been too nice, because there were a lot of these “handle null for the data item” situations.

    For example, pulling up the teacher who filed an initial report on the current (or latest) item of the student’s permanent record. (And where the customer says that almost any object in the graph could be null).

        student. permanent_record.try(:current).try(:filing_teacher).try(:last_name)

    And wow that’s:

      1. Horrible
      2. it’s hard to see where you pass parameters to #try. (Maybe you do it as extra parameters to #try, I’m not sure)

    we tried the andand gem, which offers a slightly better syntax:

        student. permanent_record.current.andand.filing_teacher.andand.last_name

    but this still has a lot of noise.

    Ultimately we were dealing with objects, not hashes (as in the original example), and we essentially implemented our own null object pattern… or more technically a null block. (It’s on my list of gems to release someday).

    We still used #try a little bit, even after that, but null checking (with try or andand) is a smell, agreed.

  3. I’m a big fan of fetch but for simple cases like these, I prefer to pass a second argument rather than using a block:

    params.fetch(:foo, {})[:bar]

    versus:

    params.fetch(:foo){ {} }[:bar]

    1. Fair enough. I prefer the consistency of always using the block form; that way if once day I replace the default with something that should only be evaluated when the key is missing, I don’t have to switch it around.

  4. What’s wrong with the built-in method of simply typing qux = params[:foo][:bar][:baz] rescue nil ?

    1. That’s a bug waiting to happen. You’re misusing exceptions to deal with a non-exceptional situation, and you’re interpreting ANY StandardError-derived exception as “missing key”. You could be accidentally masking away some exception that really should have bubbled up and been noticed.

      Nor is this merely academic: I’ve fixed plenty of bugs stemming from exactly this construct.

      “… rescue $!” is occasionally useful for capturing the exception. “… rescue nil” is just asking for trouble.

    2. That’s a bug waiting to happen. You’re misusing exceptions to deal with a non-exceptional situation, and you’re interpreting ANY StandardError-derived exception as “missing key”. You could be accidentally masking away some exception that really should have bubbled up and been noticed.

      Nor is this merely academic: I’ve fixed plenty of bugs stemming from exactly this construct.

      “… rescue $!” is occasionally useful for capturing the exception. “… rescue nil” is just asking for trouble.

  5. Cool stuff. Reminds me of Clojure’s get-in: http://clojure.github.com/clojure/clojure.core-api.html#clojure.core/get-in

    1. Nice. Somebody remind me why I even bother with Ruby anymore?

      I’m curious: not being too familiar with Clojure’s type hierarchy, is a Vector/Array considered an “associative structure” for the purpose of that function? Could you do the equivalent of the Array-inside-a-Hash fetch illustrated above?

  6. For the default value in a hash, I prefer to use a block with parameters.

    params = Hash.new { |hash, key| hash[key] = {} }
    params[:foo][:bar] = “x”
    params[:foo][:bar]      # => “x”

    params = Hash.new { {} }
    params[:foo][:bar] = “x”
    params[:foo][:bar]      # => nil

  7. Years later I found one case I cannot imagine without try()

    Given an array of objects

    a = [obj_1, obj_2, obj3]

    I want to make sure it can be serialized into a JSON object. Strings, integers, dates… do not need transformation, but structs/classes do. This is my take:

    a.map { |x| x.try(:to_hash) || x }

    and it reads in a very idiomatic way: “try to hash x, if not give me x again”

Comments are closed.