# Small: Setting locale

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

• Just a note: In Ruby, an empty string is true. Checking for params[:local].empty? is better than nil? since nil? returns false for empty strings, and the locale can be an empty string. – Mohamad Jan 30 '15 at 17:02
• @Mohamad Completely agree with you on a technical basis, but I think you need to reword your first sentence because "" == true => false – wintersolutions Jan 30 '15 at 17:07
• Sorry, I meant to say an empty string would evaluate to true, and not that it equals or returns true. Thanks for pointing that out. For exmaple: {foo: ''}[:foo].nil? => false – Mohamad Jan 30 '15 at 17:24

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.
• This is so simple and great, how could I not come up with it in the firstplace? Thanks! – wintersolutions Feb 1 '12 at 8:07

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

• Good catch there, I tought Rails would fallback to the default language if the locale wasn't found, as a quick test showed I was wrong! – wintersolutions Feb 1 '12 at 12:36