Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize Backend::Simple#available_locales #406

Merged
merged 1 commit into from Feb 9, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/i18n/backend/simple.rb
Expand Up @@ -44,7 +44,7 @@ def store_translations(locale, data, options = {})
def available_locales
init_translations unless initialized?
translations.inject([]) do |locales, (locale, data)|
locales << locale unless (data.keys - [:i18n]).empty?
locales << locale unless data.size <= 1 && (data.empty? || data.has_key?(:i18n))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please replace with if condition to make it more readable: if data.size > 1 || !(data.empty? || data.has_key?(:i18n))

Copy link
Contributor Author

@jhawthorn jhawthorn Feb 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be happy to change it, but I'm not sure that improves readability. My mental model of this block is a reject we are implementing with inject (so that we get easy locale, data pairs) and that maps best to unless.

Does an i18n maintainer have a preference?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi :) I am an i18n maintainer with an opinion on this topic.

I think unless is fine. I think the intention is clear here.

locales
end
end
Expand Down