Make sure fallback culture is always available #135
Conversation
| @@ -62,6 +63,11 @@ define(['connectionManager', 'userSettings', 'events'], function (connectionMana | |||
| for (var i in allTranslations) { | |||
| ensureTranslation(allTranslations[i], culture); | |||
| } | |||
| if (culture !== fallbackCulture) { | |||
There was a problem hiding this comment.
Why not have the if inside the loop instead?
There was a problem hiding this comment.
Because speed! :) This way it only checks once instead of how many entries there is in allTranslations.
There was a problem hiding this comment.
I doubt looping over all the entries twice is faster. Maybe it's faster if your locale is already en-us, but it seems like it would be worse for any other case. 😉
There was a problem hiding this comment.
Isn't it a moot point? :) This code executes once the application is loaded, and once a localization language is changed. Even if it adds like 50 extra milliseconds is it that bad? :)
For real, it's impossible to tell which one is faster without benchmarks, and I don't think we should really be concerned with performance here.
There was a problem hiding this comment.
I won't hold this up, but the double for loop still makes me cringe. 😄
| @@ -62,6 +63,11 @@ define(['connectionManager', 'userSettings', 'events'], function (connectionMana | |||
| for (var i in allTranslations) { | |||
| ensureTranslation(allTranslations[i], culture); | |||
| } | |||
| if (culture !== fallbackCulture) { | |||
There was a problem hiding this comment.
I doubt looping over all the entries twice is faster. Maybe it's faster if your locale is already en-us, but it seems like it would be worse for any other case. 😉
|
I know this naming was part of the code previously but "culture" feels like the wrong term to me. Shouldn't this be something like "localization" instead? |
|
@LeoVerto maybe I was sticking to the style of the file to keep changes minimal as this targeted release branch. |
Fixes #134