Getting JSON with jQuery, creating a function that displays the data from two separate feeds

I have written my first JavaScript program from scratch and am looking for advice to check whether it is efficient and effective or how it could be improved.

```
var apiURL = 'https://services.domain.com';

function dataPlanOutput(countryCode) {
document.getElementById("dataplan_list").innerHTML = "";
network_url = apiURL + '/api/v4/countries/' + countryCode ;
network_new_url = "http://cors.io/?" + network_url;
$.getJSON(network_new_url, function (networkGroup) {
$.each(networkGroup.list, function (i, list) {
var countryName = list.region;
var networkGroupId = list.networkGroupId;

function myDataPlan(networkGroupId, countryCode) {
data_url = apiURL + '/api/v4/networkGroups/' + networkGroupId + '/plansExt?countryCode=' + countryCode;
data_new_url = "http://cors.io/?" + data_url;
$.getJSON(data_new_url, function (dataPlan) {
$.each(dataPlan.list, function (i, list) {
//Price
content = '';
var currencyLetter = list.currency;
if (currencyLetter == 'USD') {
var curencySymbol = currencyLetter.replace("USD", "$");
}
else if (currencyLetter == 'GBP') {
var curencySymbol = currencyLetter.replace("GBP", "£");
}
else if (currencyLetter == 'JPY') {
var curencySymbol = currencyLetter.replace("JPY", "¥");
}
else if (currencyLetter == 'EUR') {
var curencySymbol = currencyLetter.replace("EUR", "€");
};
content += '' + curencySymbol + list.price + '

Code Cohesion

A great way to simplify code is to break things up so that each section focuses primarily on one specific task. I think your biggest problems with this are in your $.forEach(networkGroup.list loop, in which it appears you are mixing three separate tasks:

• getting the symbol

• getting the data limit

• producing html

Price: getting the symbol

Here's a quick tip to simplify converting the currencyLetter to the currencySymbol. Instead of using a list of if else blocks, it would be best to factor this part of the code out into its own function and use an object to map the keys:

function getCurrencySymbol(currencyLetter) {
  return {
    "USD": "$",
    "GBP": "£",
    "JPY": "¥",
    "EUR": "€"
  }[currencyLetter];
}

This shortens the code, makes it easy to add other currencies, and allows the function to be used in other places.

$.each(dataPlan.list, function (i, list) {
  var currencySymbol = getCurrencySymbol(list.currency); // note you misspell curencySymbol

  //Price
  content = '';
  content += '' + curencySymbol + list.price + '';
  //Data Limits
  // ...

Data Limits: getting the data limit

I'd also recommend breaking out the part of code where you find the data limit into its own function for similar reasons:

function getDataLimit(list) {
  if (list.dataUnlimited) {
    return = 'Unlimited';
  }
  else {
    var dataLimitInKB = list.dataLimitInKB;
    var measure;
    var data;

    if ((dataLimitInKB / (1024.0 * 1024.0)) >= 1.0) {
      measure = " GB";
      data = dataLimitInKB / ((1024.0) * (1024.0));
    }
    else if (dataLimitInKB / 1024.0 >= 1.0) {
      measure = " MB";
      data = dataLimitInKB / 1024.0;
    }

    data = Math.round(data * 100) / 100;
    return data + measure;
  }
}

Now the code in the $.forEach loop is focused primarily on generating the HTML which makes it much easier to see what's going on:

$.forEach() {
  //Price
  content += '' + getCurrencySymbol(list.currency) + list.price + '';

  //Data Limits
  content += '' + getDataLimit(list)+ '';

  //Data Length
  content += '' + list.validityPeriodInDays + ' Days';
  content += '';
  $(content).appendTo("#dataplan_list");
}

It would be a good idea to break up the code even more to limit how far the indentation gets. Currently, there are too many things nested inside of each other which adds a lot of complexity.

That all said, this is some pretty intense code for some of your first JavaScript. Nice work!

StackExchange Code Review Q#148703, answer score: 2