How do I avoid repeating code in a javascript for loop?

Question

I have made a form with two dropdown menus:

 <select id="selectAccountCurrency"></select>

and

 <select name="Valutatype" id="selectCurrencyType"></select>

The dropdown menus are populated with various currencies using this function:

function loadAllCurrencyKeys() {
  var keys = Object.keys(ECurrencyTypes);
  for (var index = 0; index < keys.length; index++) {
      var currencyKey = keys[index];
      var newOption = document.createElement("option");
      newOption.value = currencyKey;
      newOption.text = ECurrencyTypes[currencyKey].name;
      selectCurrencyType.options.add(newOption);
  }

for (var index = 0; index < keys.length; index++) {
      var currencyKey = keys[index];
      var newOption = document.createElement("option");
      newOption.value = currencyKey;
      newOption.text = ECurrencyTypes[currencyKey].name;
      selectAccountCurrency.options.add(newOption);
  }
}

Is there a way to make the second for loop shorter? It seems to be a lot of repetition going on there.

Additional information: Here are the object I'm trying to list in both dropdown menus at the same time:

  var ECurrencyTypes = {
NOK: {value:1.00000, name: "Norske kroner", denomination: "kr"},
EUR: {value:0.10733, name: "Europeiske euro", denomination: "€"},
USD: {value:0.12652, name: "United States dollar", denomination: "$"},
GBP: {value:0.09550, name: "Pound sterling", denomination: "£"},
INR: {value:8.18624, name: "Indiske rupee", denomination: "र"},
AUD: {value:0.16129, name: "Australienske dollar", denomination: "A$"},
PHP: {value:6.48595, name: "Filippinske peso", denomination: "₱"},
SEK: {value:1.02580, name: "Svenske kroner", denomination: "kr"},
CAD: {value:0.15841, name: "Canadiske dollar", denomination: "C$"},
THB: {value:4.18410, name: "Thai baht", denomination: "฿"}
};

And this is some of the html-code with the fieldsets and select-tags:

<fieldset>
    <label for="txtAmount">Amount: </label>
    <input type="text" id="txtAmount">
    <label for="selectCurrencyType"> in </label>
    <select name="Valutatype" id="selectCurrencyType"></select>
</fieldset>

and

<fieldset>
    <label for="selectAccountCurrency">Currency:</label>
    <select id="selectAccountCurrency"></select>
</fieldset>

Does your code work? If so, this would be a better question for Code Review — GrumpyCrouton
– GrumpyCrouton, Commented Jan 10, 2019 at 19:42
Why not put this line selectAccountCurrency.options.add(newOption); under selectCurrencyType.options.add(newOption); and remove the second loop? — The fourth bird
– The fourth bird, Commented Jan 10, 2019 at 19:43
@Thefourthbird Was just about to say that, the entire 2nd loop is not needed — GrumpyCrouton
– GrumpyCrouton, Commented Jan 10, 2019 at 19:43
@Thefourthbird because then the DOM would remove the element from the first select's options when placing it in the second one — Patrick Roberts
– Patrick Roberts, Commented Jan 10, 2019 at 19:52

quirimmo · Accepted Answer · 2019-01-10 19:52:53Z

1

You have the second loop just becase you want to add the same option to both the select, but they need to be two different HTML elements.

Use just one loop, clone the element, and then append one to the first select and the cloned one to the other select, so you have just one loop, and they will be two different HTML elements, rather than the same one:

function loadAllCurrencyKeys() {
  var keys = Object.keys(ECurrencyTypes);
  for (var index = 0; index < keys.length; index++) {
      var currencyKey = keys[index];
      var newOption = document.createElement("option");
      newOption.value = currencyKey;
      newOption.text = ECurrencyTypes[currencyKey].name;
      selectCurrencyType.options.add(newOption);
      selectAccountCurrency.options.add(newOption.clone(true)); // <- see the .clone
  }
}

answered Jan 10, 2019 at 19:52

quirimmo

10k3 gold badges33 silver badges46 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Patrick Roberts · Accepted Answer · 2019-01-10 20:29:12Z

All the other answers seem to be correct, but you can make your code a bit less verbose by using Object.entries() and destructuring syntax in a for...of loop:

function loadAllCurrencyKeys() {
  for (const [key, value] of Object.entries(ECurrencyTypes)) {
    const newOption = document.createElement("option");
    newOption.value = key;
    newOption.text = value;
    selectCurrencyType.options.add(newOption);
    selectAccountCurrency.options.add(newOption.clone(true));
  }
}

And if you have more than two <select> elements, you can iterate those as well by passing them to the function:

function loadAllCurrencyKeys(...selects) {
  for (const [key, value] of Object.entries(ECurrencyTypes)) {
    let newOption = document.createElement("option");
    newOption.value = key;
    newOption.text = value;

    for (const select of selects) {
      select.options.add(newOption);
      newOption = newOption.clone(true);
    }
  }
}

loadAllCurrencyKeys(selectCurrencyType, selectAccountCurrency);

The only downside being that the <option> is cloned one more time than necessary for each currency type, though that will most likely be trivial overhead.

Sushanth -- · Accepted Answer · 2019-01-10 19:52:41Z

0

You can just use a single for loop and then clone the newOption that was created before inserting it into the 2nd dropdown.

function loadAllCurrencyKeys() {
  var keys = Object.keys(ECurrencyTypes);
  for (var index = 0; index < keys.length; index++) {
    var currencyKey = keys[index];
    var newOption = document.createElement("option");
    newOption.value = currencyKey;
    newOption.text = ECurrencyTypes[currencyKey].name;
    
    selectCurrencyType.options.add(newOption);
    
    var clonedNewOption = newOption.clone(true);
    
    selectAccountCurrency.options.add(clonedNewOption);
  }
}

answered Jan 10, 2019 at 19:52

Sushanth --

55.8k9 gold badges70 silver badges109 bronze badges

1 Comment

Senseless Over a year ago

Thank you for all the answers and effort. I just can't get the dropdown menu to appear at both places at the same time. Either the menu just appears at one place or most of options disappear and I'm left with only "Norske kroner". I want both select-fields to provide the same options, and preferably get the options from the var ECurrencyTypes.

Fahim Farook · Accepted Answer · 2019-01-10 19:53:11Z

If the dropdown values are different;

function loadAllCurrencyKeys(data, select) {
      var keys = Object.keys(data);
      for (var index = 0; index < keys.length; index++) {
          var currencyKey = keys[index];
          var newOption = document.createElement("option");
          newOption.value = currencyKey;
          newOption.text = ECurrencyTypes[currencyKey].name;
          select.options.add(newOption);
      }
    }

and elsewhere in the code;

loadAllCurrencyKeys(ECurrencyTypes, selectCurrencyType);
loadAllCurrencyKeys(ECurrencyTypes, selectAccountCurrency);

And if the dropdown values are the same for both the dropdowns, then you may hardcode the dropdown elements in the for loop. i.e.

selectCurrencyType.options.add(newOption);
selectAccountCurrency.options.add(newOption);

Collectives™ on Stack Overflow

How do I avoid repeating code in a javascript for loop?

4 Answers 4

Comments

Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related