The natural data structure for looking up things is a map. In JavaScript this is simply an object and is written like {key: value}
. It provides efficient access.
Therefore my first idea was to use two maps for the two kinds of lookup you need: country by ISO-3166-alpha2 and country by ISO-3166-alpha3. So instead of your single countries
map, I prefer to have these two maps:
const countryByIso2 = {};
const countryByIso3 = {};
These are currently empty but will be filled soon.
function init() {
const countryCodes = [
['US', 'USA'],
['UK', 'GBR'],
['CA', 'CAN'],
['AU', 'AUS'],
['PR', 'PRI'],
['GB', 'GBR']
];
const overrides = [
['GB', 'UK'],
['CA', 'US']
];
for (let country of countryCodes) {
countryByIso2[country[0]] = country;
countryByIso3[country[1]] = country;
}
for (let override of overrides) {
let from = countryByIso2[override[0]];
let to = countryByIso2[override[1]];
countryByIso2[from[0]] = countryByIso2[to[0]];
countryByIso3[from[1]] = countryByIso3[to[1]];
}
}
init();
In this function I have removed all the unnecessary data and left just the country codes. There are two configuration sections. The first is the mapping you already know. The second are the overrides that you wrote in the if
clauses of your getCountry
function.
After the first part of the initialization, countryByIso2['CA']
contains ['CA', 'CAN']
. But when the overrides are done, they overwrite the existing map entries. This means that after the initialization you can inspect the countryByIso2
object and quickly see what you will get.
What's missing is the actual lookup.
function lookupCountry(code) {
if (!code) {
return null;
}
let upperCode = code.toUpperCase();
return countryByIso2[upperCode]
|| countryByIso3[upperCode];
}
function getCountry(i18nHeader, i18nQuery) {
return lookupCountry(i18nHeader)
|| lookupCountry(i18nQuery)
|| countryByIso2['US'];
}
In total, this code is longer than yours, but it uses only very few and very fast operations. It is suitable for country lists of several thousand. Your code on the other hand walks through all entries (lookupCountry
) and takes a long time when the data grows.
There's another difference. If i18nHeader
is set to some unknown value, your code returns undefined
while my code returns the US country codes.
The next topic is putting your code into its own namespace. After the above definitions, all parts of the code (even those you didn't write) can access the init
function, the countryByIso2
tables, the lookupCountry
function. But these are meant to be internal to your code and should not be seen by any unrelated code.
Therefore, it is best practice to explicitly define those pieces of code you want to export to the other code and only make that visible. This is typically done with the following pattern, putting all your code into an anonymous function and then returning the things to be exported:
const getCountry = (function() {
const countryByIso2 = {};
const countryByIso3 = {};
const countryCodes = [
['US', 'USA'],
['UK', 'GBR'],
['CA', 'CAN'],
['AU', 'AUS'],
['PR', 'PRI'],
['GB', 'GBR']
];
const overrides = [
['GB', 'UK'],
['CA', 'US']
];
for (let country of countryCodes) {
countryByIso2[country[0]] = country;
countryByIso3[country[1]] = country;
}
for (let override of overrides) {
let from = countryByIso2[override[0]];
let to = countryByIso2[override[1]];
countryByIso2[from[0]] = countryByIso2[to[0]];
countryByIso3[from[1]] = countryByIso3[to[1]];
}
function lookupCountry(code) {
if (!code) {
return null;
}
let upperCode = code.toUpperCase();
return countryByIso2[upperCode]
|| countryByIso3[upperCode];
}
function getCountry(i18nHeader, i18nQuery) {
return lookupCountry(i18nHeader)
|| lookupCountry(i18nQuery)
|| countryByIso2['US'];
}
return getCountry;
})();
When I saw your code, the first thing I did was to remove the unnecessary else
clauses from the getCountry
function. Then it looked like this:
function getCountry(i18nHeader, i18nQuery) {
if (i18nQuery) {
return lookupCountry(countries, i18nQuery);
}
if (!i18nHeader || !i18nHeader.length) {
return countries.UNITED_STATES;
}
if (i18nHeader.toUpperCase() === countries.GREAT_BRITAIN[0]) {
return countries.UNITED_KINGDOM;
}
if (i18nHeader.toUpperCase() === countries.CANADA[0]) {
return countries.UNITED_STATES;
}
return lookupCountry(countries, i18nHeader);
}
It's a one-dimensional chain of decisions now, instead of a hard-to-grasp nested decision tree. It's still difficult to follow, and that's why I rewrote it in the alternative way described above.
i18nHeader
but not for thei18nQuery
? \$\endgroup\$ – Roland Illig Jan 22 '18 at 22:45Map
object, not regular objects. developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/… \$\endgroup\$ – Aaron Goldsmith Jan 27 '18 at 0:28