Small: Setting locale

Question

I'm new to Rails/Ruby and I would like to get feedback on my little ApplicationController which is able to detect the visitor's language. I'm especially interested in optimizations on the control structure in the set_locale method (2 nested if statements and 3 assignments to one variable). I'm using the http_accept_language gem.

class ApplicationController < ActionController::Base
  AVAILABLE_LANGUAGES = %w{de en}

  before_filter :set_locale
  protect_from_forgery


  def set_locale
    if params[:locale].nil?
      if preferred_language = request.preferred_language_from(AVAILABLE_LANGUAGES)
        I18n.locale = preferred_language
      else
        I18n.locale = I18n.default_locale
      end
    else
      I18n.locale = params[:locale]
    end
  end

  def default_url_options(options={})
    logger.debug "default_url_options is passed options: #{options.inspect}\n"
    { :locale => I18n.locale }
  end
end

Answer 1

You can do it in an "one-liner". Although one-liners are frequently associated with less readability, I honestly believe this is not the case. Something like this would work:

I18n.locale = params[:locale] || 
              request.preferred_language_from(AVAILABLE_LANGUAGES) ||
              I18n.default_locale

I see a few advantages in this solution:

• Your code will be a bit cleaner, since you can collapse 9 lines of code filled with if/else conditions into 3 (or 1, if you don't break the line);
• You won't need to allocate preferred_language in case params[:locale] is nil;
• And the biggest advantage of them all: clarity. The precedence of the options is much more explicit.

Answer 2

I like the one liner from goncalossilva, but I would probably add a check to the first clause.

def valid_locale(locale)
  locale if AVAILABLE_LANGUAGES.include?(locale)
end


I18n.locale = valid_locale(params[:locale]) || 
              request.preferred_language_from(AVAILABLE_LANGUAGES) ||
              I18n.default_locale