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 shiftunshift 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.
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 shift 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.
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.
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:
I18n.locale = [user_locale, browser_locale, param_locale, I18n.default_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 gotgo through the list, until it finds a 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 shift 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.
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 value.
However, you're looking to prioritize locale selection, in which case I'd do something like:
I18n.locale = [user_locale, browser_locale, param_locale, I18n.default_locale]
.compact
.map(&:to_sym)
.detect { |locale| I18n.available_locales.include?(locale) }
cookies.permanent[:my_locale] = I18n.locale
It'll got through the list, until it finds a locale that's not nil and is available.
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 shift 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.
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 shift 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.
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 value.
However, you're looking to prioritize locale selection, in which case I'd do something like:
I18n.locale = [user_locale, browser_locale, param_locale, I18n.default_locale]
.compact
.map(&:to_sym)
.detect { |locale| I18n.available_locales.include?(locale) }
cookies.permanent[:my_locale] = I18n.locale
It'll got through the list, until it finds a locale that's not nil and is available.
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 shift 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.