For loop creating Options

Question

I have a Select HTML Component to which i am adding Options dynamically

This is my code , where the functionality is working fine

Based on the selectedValue i am adding the selected attribute dynamically to the Option

Is there any better way of adding the selected attribute

$(document).ready(function() {
    var option = '';
    var cars = ["Saab", "Volvo", "BMW"];
    var selectedValue = 'Volvo'

        for (var i = 0; i < cars.length; i++) {
            if (selectedValue && cars[i] == selectedValue)
                option += '<option value="' + cars[i] + '" selected>' + cars[i] + '</option>';
            else
                option += '<option value="' + cars[i] + '">' + cars[i] + '</option>';
        }

    $('#carscomponent').append(option);
});

https://jsfiddle.net/wmLsv94x/3/

This may be better suited to https://codereview.stackexchange.com/ but personally I wouldn't build the elements as a string, but something more like this: https://stackoverflow.com/a/4158203/1650337 — DBS, Feb 20 '19 at 10:13
@DBS that would depend on your goal. The OPs method of building a string and appending once is much quicker than creating individual Element/jQuery objects, however the latter is (arguably) cleaner and easier to maintain. — Rory McCrossan, Feb 20 '19 at 10:14
Different people will give different answers. The only suggestion I would make is: Try to cut down on duplication. My personal approach would be to create an extra variable like `var selected = (selectedValue && cars[i] == selectedValue) ? ' selected="selected"' : '';` and concatonate, rather than having the entire tag defined twice. — Scoots, Feb 20 '19 at 13:32

score 0 · Answer 1 · answered Feb 20 '19 at 13:42

This is what I would do to optimize the code.

Get rid of unnecessary jQuery.
Create option elements using the DOM API rather than working with strings.

document.addEventListener('DOMContentLoaded', function() {
  const options = [];
  const cars = ["Saab", "Volvo", "BMW"];
  const selectedValue = 'Volvo';
  const carsComponent = document.getElementById('carsComponent');

  for (let car of cars) {
    let option = document.createElement('option');
    option.selected = car === selectedValue;
    option.value = car;
    option.textContent = car;
    options.push(option)
  }
  
  for (let option of options) {
    carsComponent.appendChild(option);
  }
});

<select id="carsComponent"></select>

If you want to further optimize (performance-wise), then use a documentFragment to append the single options first, then use appendChild on the select with that fragment's content.

For loop creating Options

1 Answers1