group countries by language in JavaScript

Question

I need a better implementation than the below code (O(N) Solution), I am grouping countries by language ( data ) is the countries JSON, each country could have 0, 1 .. or more language as the object key, value where the key is the language shortcut and the value is the language official name.

module.exports.groupByLanguage = async (data, grouped) => {
  try {
    data.forEach((country) => {
      if (country.languages) {
        let countryLanguages = Object.keys(country.languages)
        countryLanguages.forEach((language) => {
          const keys = Object.keys(grouped.languages)
          if (language.toString() in grouped.languages) {
            grouped.languages[language].push(country)
          } else {
            grouped.languages[language] = []
            grouped.languages[language].push(country)
          }
        })
      } else {
        grouped.languages['notEntries'].push(country)
      }
    })
  } catch (err) {
    console.log(err)
  }

  return grouped
}

JSON for each country

{
  "name": {},
  "codes": {
    "cca2": "MT",
    "cca3": "MLT",
    "ccn3": "470"
  },
  "languages": {
    "eng": "English",
    "mlt": "Maltese"
  },
  "currencies": {
    "EUR": {
      "name": "Euro",
      "symbol": "€"
    }
  },
  "region": "Europe",
  "latlng": [35.83333333, 14.58333333]
}

check out the results Here

Answer 1

Code should be correct, maintainable, robust, reasonably efficient, and, most important, readable.

---

Your code reads as a tangled web of nested conditionals.

It's not clear that 'notEntries' always exists.

Catch error handling merely writes a message to the console.log.

You have not provided test input data.

---

Let's use stepwise refinement: Program Development by Stepwise Refinement by Niklaus Wirth.

NOTE: This is a simple first draft, without the benefit of test data, to illustrate program development.

---

Code should reflect the main path of the problem solution. Later, we add code for exceptions and errors.

const groupByLanguage = (countries) => {
    let byLanguage = {};

    countries.forEach((country) => {
        let languages = country.languages;

        const languageCodes = Object.keys(languages);
        languageCodes.forEach((languageCode) => {
            let byLanguageCode = byLanguage[languageCode];
            byLanguageCode.push(country);
        });
    });

    return {"languages": byLanguage};
}

---

Now we add code for exceptions and errors without obscuring the main path of the code.

const groupByLanguage = (countries) => {
    let byLanguage = {};

    countries.forEach((country) => {
        let languages = country.languages;
        if (languages === undefined) {
            languages = {
                "none": "None",
            };
        }

        const languageCodes = Object.keys(languages);
        languageCodes.forEach((languageCode) => {
            let byLanguageCode = byLanguage[languageCode];
            if (byLanguageCode === undefined) {
                byLanguage[languageCode] = [];
                byLanguageCode = byLanguage[languageCode];
            }
            byLanguageCode.push(country);
        });
    });

    return {"languages": byLanguage};
}

---

If useful, we convert the function to an async function.

const groupByLanguage = async (countries) => {
    let byLanguage = {};

    countries.forEach((country) => {
        let languages = country.languages;
        if (languages === undefined) {
            languages = {
                "none": "None",
            };
        }

        const languageCodes = Object.keys(languages);
        languageCodes.forEach((languageCode) => {
            let byLanguageCode = byLanguage[languageCode];
            if (byLanguageCode === undefined) {
                byLanguage[languageCode] = [];
                byLanguageCode = byLanguage[languageCode];
            }
            byLanguageCode.push(country);
        });
    });

    return {"languages": byLanguage};
}

Answer 2

To add to Peter's answer, it is worth thinking about creating a data structure to support what you need, instead of managing it yourself. I almost always find myself making some type dict these types of situations, which makes the code cleaner.

class ArrayMap {
  add(code, country) {
    this[code] ??= []
    this[code].push(country)
  }
}

This greatly simplifies the code, because it gets rid of the clutter that is introduced with handling default values on the result structure.

function languages(country) {
  if(!country.languages) return undefined

  return Object.keys(country.languages)
}

function groupByLanguage(countries) {
  return countries.reduce((languageMap, country) => {
    const languages = languages(country) ?? ['nonEntries']
      
    languages.forEach(language => languageMap.add(language, country))

    return languageMap
  }, new ArrayMap())
}

Another thing to point out about your code is that you are passing "grouped" object in as a parameter. This is not that great, because you are mutating it. The resulting code is almost unmaintainable, because you have no idea what is happening inside the function with that object. It is a rule of thumb that each time you call a function that returns something, it should have no side effects (it shouldn't do mutations to parameters).

So instead of groupByLanguage(countries, grouped) it is way more readable to do grouped.languages = groupByLanguage(countries).