HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyrailsMinor

Setting locale based on preferences with fallbacks

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
preferencesfallbackswithsettinglocalebased

Problem

Consider the following code. Are the multiple assignments to the loc variable a code smell? If so, how can I refactor this?

def set_locale
  begin
    loc = user_signed_in? ? user_locale : browser_locale
    loc = browser_locale if loc.nil?
    loc = param_locale if loc.nil?
    loc = I18n.available_locales.include?(loc.to_sym) ? loc.to_sym : I18n.default_locale
  rescue
    loc = I18n.default_locale
  ensure
    cookies.permanent[:my_locale] = loc
  end
  I18n.locale = loc
end

Solution

I'd say it's a code smell. Mostly because loc seems to change type. It's one thing for it to be nil or a symbol; in that case it might "only" change value. But the last to_sym would seem to suggest it could also be a string or something.

In terms of code style, you could just use the ||= assignment operator instead of x = y if x.nil?. That's the Ruby way to provide a fallback/default value in case of nil.

However, you're looking to prioritize locale selection, in which case I'd do something like:

locale = [user_locale, browser_locale, param_locale]
  .compact
  .map(&:to_sym)
  .detect { |locale| I18n.available_locales.include?(locale) }

I18n.locale = locale || I18n.default_local
cookies.permanent[:my_locale] = I18n.locale


It'll go through the list and return the first locale that's not nil and is available. If nothing's returned, it uses the default locale.

Note that I'm making an assumption here, namely that user_locale will return nil if the user isn't signed in. But you could also simply unshift the user_locale onto the array if the user is signed in, before doing the detect.

On another note: param_locale should, I think, be the first choice in all cases. If I go to a URL that has an explicit locale code in it, I'd expect to see exactly that page, in that language. URLs should be a permanent as you can make them. Sure, give me an option to view the same URL with a different locale, but don't second guess the URL itself.

Code Snippets

locale = [user_locale, browser_locale, param_locale]
  .compact
  .map(&:to_sym)
  .detect { |locale| I18n.available_locales.include?(locale) }

I18n.locale = locale || I18n.default_local
cookies.permanent[:my_locale] = I18n.locale

Context

StackExchange Code Review Q#108505, answer score: 3

Revisions (0)

No revisions yet.