gistfile1.rb

LOOKUP_CHAIN = [:params, :user, :session, :http, :default]

def self.set_by(inputs)
  locale = nil
  LOOKUP_CHAIN.each do |lookup|
    locale = send("by_#{lookup}", inputs[lookup])
    break if locale
  end
  return locale
end

@gicappa replace the map with detect and it will return nil if none are found.
That looks like the sanest way to write it.

@tyre You can't change the map to detect because then it will return the first value in LOOKUP_CHAIN with a truthy return value, but not the actual result of evaluating the block itself. You'll be getting a symbol from LOOKUP_CHAIN as your final result, versus the actual locale.

Oh yes that makes perfect sense. In that case an explicit return unless the send is nil? ``` ruby { (locale = send(...)) && return locale } ```

…

On Friday, August 3, 2012 at 7:47 AM, Ernie Miller wrote: @tyre You can't change the map to detect because then it will return the first value in LOOKUP_CHAIN with a truthy return value, but not the actual result of evaluating the block itself. You'll be getting a symbol from LOOKUP_CHAIN as your final result, versus the actual locale. --- Reply to this email directly or view it on GitHub: https://gist.github.com/3247167

A test harness of sorts, so that we can verify that our changed versions match the original:

class LocaleChooser


    LOOKUP_CHAIN = [:params, :user, :session, :http, :default]

    def self.set_by(inputs)
        locale = nil
        LOOKUP_CHAIN.each do |lookup|
            locale = send("by_#{lookup}", inputs[lookup])
            break if locale
        end
        return locale
    end


    # define self.by_params(input), self.by_user(input), etc.
    class << self
        returning_lookups = [:user, :http] # edit me for testing

        LOOKUP_CHAIN.each do |lookup|
            define_method("by_#{lookup}") do |input|
                puts "evaluated by_#{lookup}"

                if returning_lookups.include?(lookup)
                    return [lookup, input]
                else
                    return nil
                end
            end
        end
    end
end

p LocaleChooser.set_by({})
puts '---'
p LocaleChooser.set_by(:user => :custom_user) 
puts '---'
p LocaleChooser.set_by(:default => :custom_default)
puts '---'
p LocaleChooser.set_by(:params => :custom_params)

This harness is based on the code in ernie’s comment. The main part of the code, separated with whitespace, is straight from this Gist.

If you run this as shown above (on Ruby 1.9), you get this output:

evaluated by_params
evaluated by_user
[:user, nil]

---
evaluated by_params
evaluated by_user
[:user, :custom_user]

---
evaluated by_params
evaluated by_user
[:user, nil]

---
evaluated by_params
evaluated by_user
[:user, nil]

Change the calls at the bottom and the contents of returning_lookups to test different behaviors.

Here’s a refactoring:

module Enumerable
  def find_mapped
    result = nil
    self.each do |lookup|
      result = yield(lookup)
      break if result
    end
    return result
  end
end

LOOKUP_CHAIN = [:params, :user, :session, :http, :default]

def self.set_by(inputs)
  LOOKUP_CHAIN.find_mapped do |lookup|
    locale = send("by_#{lookup}", inputs[lookup])
  end
end

It produces the same output in the test harness I posted previously.

Oh, and sorry for the 4-space indentation in that previous test-harness code. I forgot to convert from tabs to spaces, and GitHub won’t let me edit that comment.

I made a Gist fork for my refactoring that I just posted.

I just found out that you can make more than one fork of a Gist. So here’s a Gist fork for the test harness I posted earlier.

This Gist was posted alongside a tweet by the author:

Any ideas to refactor this little ruby collection operation? https://gist.github.com/3247167

Thought about this the other day but just got around to trying it this afternoon. I believe #any? stops when it reaches a truthy result, so how about this:

def self.set_by(inputs)
  locale = nil
  LOOKUP_CHAIN.any? { |lookup| locale = send("by_#{lookup}", inputs[lookup]) }
  return locale
end

Passes the test harness.