Converting repetative javascript statements to arrays in loops

Question

I am dynamically changing the options in 8 selects based on a separate selected option. The process handles counties in a state and updating all 8 selects for each county in a state is terribly repetitive as each option can only exist in one select.

My original code was thrown together quickly but worked fine:

FLCounties = [
"Alachua",
"Baker",
"Bay",
"etc."];

//Get the selects
var countyDrop1 = document.getElementById("county1");
var countyDrop2 = document.getElementById("county2");
var countyDrop3 = document.getElementById("county3");
var countyDrop4 = document.getElementById("county4");
var countyDrop5 = document.getElementById("county5");
var countyDrop6 = document.getElementById("county6");
var countyDrop7 = document.getElementById("county7");
var countyDrop8 = document.getElementById("county8");

//Get the default state if any
var initialState = document.getElementById("state").value;
getState(initialState);

//Called from onchange on the state select
function getState(sel) {

//Clear previous options and add a notice to select a state
while (countyDrop1.firstChild) {
    countyDrop1.removeChild(countyDrop1.firstChild);
};
    var defaultEl1= document.createElement("option");
    defaultEl1.textContent = "Select a state above first.";
    defaultEl1.value = "";
    countyDrop1.appendChild(defaultEl1);

while (countyDrop2.firstChild) {
    countyDrop2.removeChild(countyDrop2.firstChild);
};
    var defaultEl2= document.createElement("option");
    defaultEl2.textContent = "Select a state above first.";
    defaultEl2.value = "";
    countyDrop2.appendChild(defaultEl2);

while (countyDrop3.firstChild) {
    countyDrop3.removeChild(countyDrop3.firstChild);
};
    var defaultEl3= document.createElement("option");
    defaultEl3.textContent = "Select a state above first.";
    defaultEl3.value = "";
    countyDrop3.appendChild(defaultEl3);

while (countyDrop4.firstChild) {
    countyDrop4.removeChild(countyDrop4.firstChild);
};
    var defaultEl4= document.createElement("option");
    defaultEl4.textContent = "Select a state above first.";
    defaultEl4.value = "";
    countyDrop4.appendChild(defaultEl4);

while (countyDrop5.firstChild) {
    countyDrop5.removeChild(countyDrop5.firstChild);
};
    var defaultEl5= document.createElement("option");
    defaultEl5.textContent = "Select a state above first.";
    defaultEl5.value = "";
    countyDrop5.appendChild(defaultEl5);

while (countyDrop6.firstChild) {
    countyDrop6.removeChild(countyDrop6.firstChild);
};
    var defaultEl6= document.createElement("option");
    defaultEl6.textContent = "Select a state above first.";
    defaultEl6.value = "";
    countyDrop6.appendChild(defaultEl6);

while (countyDrop7.firstChild) {
    countyDrop7.removeChild(countyDrop7.firstChild);
};
    var defaultEl7= document.createElement("option");
    defaultEl7.textContent = "Select a state above first.";
    defaultEl7.value = "";
    countyDrop7.appendChild(defaultEl7);

while (countyDrop8.firstChild) {
    countyDrop8.removeChild(countyDrop8.firstChild);
};
    var defaultEl8= document.createElement("option");
    defaultEl8.textContent = "Select a state above first.";
    defaultEl8.value = "";
    countyDrop8.appendChild(defaultEl8);

//if no state is selected, do nothing  
if (sel != ""){
    switch (sel){
        //value of option Florida
        case "FL":
            //change "select a state first" to "select a county"
            countyDrop1.firstChild.textContent = "- Select a County -";
            countyDrop2.firstChild.textContent = "- Select a County -";
            countyDrop3.firstChild.textContent = "- Select a County -";
            countyDrop4.firstChild.textContent = "- Select a County -";
            countyDrop5.firstChild.textContent = "- Select a County -";
            countyDrop6.firstChild.textContent = "- Select a County -";
            countyDrop7.firstChild.textContent = "- Select a County -";
            countyDrop8.firstChild.textContent = "- Select a County -";
            for(var i = 0; i < FLCounties.length; i++) {

                var opt = FLCounties[i];
                var el1 = document.createElement("option");
                el1.textContent = opt;
                el1.value = opt;
                var el2 = document.createElement("option");
                el2.textContent = opt;
                el2.value = opt;
                var el3 = document.createElement("option");
                el3.textContent = opt;
                el3.value = opt;
                var el4 = document.createElement("option");
                el4.textContent = opt;
                el4.value = opt;
                var el5 = document.createElement("option");
                el5.textContent = opt;
                el5.value = opt;
                var el6 = document.createElement("option");
                el6.textContent = opt;
                el6.value = opt;
                var el7 = document.createElement("option");
                el7.textContent = opt;
                el7.value = opt;
                var el8 = document.createElement("option");
                el8.textContent = opt;
                el8.value = opt;
                countyDrop1.appendChild(el1);
                countyDrop2.appendChild(el2);
                countyDrop3.appendChild(el3);
                countyDrop4.appendChild(el4);
                countyDrop5.appendChild(el5);
                countyDrop6.appendChild(el6);
                countyDrop7.appendChild(el7);
                countyDrop8.appendChild(el8);
            }
            break;
            [rinse and repeat, 49 times]

I tried condensing it into a series of arrays and for loops, which really shortens the code, but it breaks the page (an infinite loop somewhere I'm sure).

New code (does not work):

var FLCounties = [
//same as above
];

var dropdowns = [
countyDrop1 = document.getElementById("county1"),
countyDrop2 = document.getElementById("county2"),
countyDrop3 = document.getElementById("county3"),
countyDrop4 = document.getElementById("county4"),
countyDrop5 = document.getElementById("county5"),
countyDrop6 = document.getElementById("county6"),
countyDrop7 = document.getElementById("county7"),
countyDrop8 = document.getElementById("county8")
];

var defaultOptions = [
defaultEl1 = document.createElement("option"),
defaultEl2 = document.createElement("option"),
defaultEl3 = document.createElement("option"),
defaultEl4 = document.createElement("option"),
defaultEl5 = document.createElement("option"),
defaultEl6 = document.createElement("option"),
defaultEl7 = document.createElement("option"),
defaultEl8 = document.createElement("option")
];

var options = [
el1 = document.createElement("option"),
el2 = document.createElement("option"),
el3 = document.createElement("option"),
el4 = document.createElement("option"),
el5 = document.createElement("option"),
el6 = document.createElement("option"),
el7 = document.createElement("option"),
el8 = document.createElement("option")
];

var initialState = document.getElementById("state").value;
getState(initialState);

function getState(sel) {
    //new clear list
    for(i=0;i<8;i++){
        while (dropdowns[i].firstChild) {
            dropdowns[i].removeChild(dropdowns[i].firstChild);
        }
        defaultOptions[i].textContent = "Select a state above first.";
        defaultOptions[i].value = "";
        dropdowns[i].appendChild(defaultOptions[i]);
    }
    if (sel != ""){
        var opt;
        switch (sel){
            case "FL":
                //new change "select a state first" to "select a county"
                for (i=0;i<8;i++){
                    dropdowns[i].firstChild.textContent = "- Select a County -";
                }
                //new create 8 unique options and add them to the selects
                for(i=0;i<FLCounties.length;i++){
                    opt = FLCounties[i];
                    for (i=0;i<8;i++){
                        options[i].textContent = opt;
                        options[i].value = opt;
                        for(i=0;i<8;i++){
                            dropdowns[i].appendChild(options[i]);
                        }
                    }
                }
            break;
        }
    }
}

Am I missing something? This should execute quicker right? Instead its holding the page from loading indefinitely.

Edit: I did find one glaring problem. The dropdowns[i].appendChild(options[i]); loop should not be in the loop above it. It should run after options has been set.

New snippet:

//new create 8 unique options and add them to the selects
for(i=0;i<FLCounties.length;i++){
    opt = FLCounties[i];
    for (i=0;i<8;i++){
        options[i].textContent = opt;
        options[i].value = opt;
    }            
    for(i=0;i<8;i++){
        dropdowns[i].appendChild(options[i]);
     }
 }

Edit 2:

I ran into a problem when taking a step back. It stems from using the array of options in a loop. I knew each option element could only exist in a select once, but didn't follow that logic when writing the loop. The same option element (options[i]) is being changed and reused in each iteration, which wont work, you only get the last iteration's results. I believe this worked in the old code because it redefined the element in each iteration, not just changed it's attributes

Does this rule an array out for this use?

First thing to note: var options = [ el1 = document.createElement("option"), is probably not doing what you think it is. If you want an associative array aka object or hash: var options = { el1: document.createElement("options"), … — jacob, Dec 19 '13 at 7:10
Is valid syntax I think he's just creating bunch of globals. — elclanrs, Dec 19 '13 at 7:11
I would also kill the repeated code when you specify the arrays. For example, for( var i=1; i<=8; i++ ) dropdowns.push( document.getElementById( 'county' + i ) ); — Ethan Brown, Dec 19 '13 at 7:11
And yet it seems like the user was able to execute it correctly. As he says My original code was thrown together quickly but worked fine: — Akshay Khandelwal, Dec 19 '13 at 7:11
I was thinking I needed to declare them first then add them to the array. Is that right? — Thatoneguy7, Dec 19 '13 at 7:12

jasonscript · Answer 1 · 2013-12-19 07:10:15Z

up vote 1 down vote

That's not how you add elements to an array

var dropdowns = [
    countyDrop1 = document.getElementById("county1"),
    countyDrop2 = document.getElementById("county2"),
    countyDrop3 = document.getElementById("county3")
];

should be

var dropdowns = [
    document.getElementById("county1"),
    document.getElementById("county2"),
    document.getElementById("county3")
];

which you would then access like so

dropdown[0] // gives county1

you may want to use an object instead (note the difference [] becomes {})

var dropdowns = {
    'county1':document.getElementById("county1"),
    'county2':document.getElementById("county2"),
    'county3':document.getElementById("county3")
};

which you can then access like so

dropdown.county1

answered Dec 19 '13 at 7:10

jasonscript
877311

5

And thats not how you answer a question in SO :p – thefourtheye Dec 19 '13 at 7:10

can you not define them in the array's define? – Thatoneguy7 Dec 19 '13 at 7:14

@thefourtheye accidental hit of the add comment button. See updated answer – jasonscript Dec 19 '13 at 7:14

I took out the variable names, as I wont need them. The script still holds the page from rendering. – Thatoneguy7 Dec 19 '13 at 7:18

Is there an advantage to objects here? besides being able to refer to the variables in the object by name that is. – Thatoneguy7 Dec 19 '13 at 7:40

show 2 more comments

Thatoneguy7 · Answer 2 · 2013-12-23 19:56:13Z

As I noted in the comments above, when adding the options to the select, I cannot use an array. II ended up using a two dimensional array for the counties, and a second array for the state codes that my php script submits to the form.

I realize that the arrays would be better defined as objects, but the deadline for the project is up and this code is efficient enough for production use. Here is a condensed version of my final code (until things slow down and I find time to revisit the code):

ALCounties = ["Autauga","Baldwin","Barbour","Bibb",etc];
AKCounties = ["Anchorage Borough","Bethel Census Area",etc];
etc

stateCountyList = [ALCounties,AKCounties,etc];

stateCode = [ "AL","AK","AZ","AR",etc];

countyDrops = [
    document.getElementById("county1"),
    document.getElementById("county2"),
    document.getElementById("county3"),
    document.getElementById("county4"),
    document.getElementById("county5"),
    document.getElementById("county6"),
    document.getElementById("county7"),
    document.getElementById("county8")
];

defaultOpts = [
    document.createElement("option"),
    document.createElement("option"),
    document.createElement("option"),
    document.createElement("option"),
    document.createElement("option"),
    document.createElement("option"),
    document.createElement("option"),
    document.createElement("option")
];

//If a user loads the page with a state previously stored in the db
var initialState = document.getElementById("state").value;
getState(initialState);

// Replaces the switch in the old code.
// Passed value of option in state select on the page, matches it to a var in stateCode, calls updateState to create a list for that state
function getState(sel) {
    clearSetFirst(" - Select a state above first. - ");
    if (sel != ""){
        var valid = false;
        for (i=0;i<stateCode.length;i++){
            if(sel == stateCode[i]){
                updateState(stateCountyList[i]);
                valid = true;
                break;
            }
        }
        if(!valid){
            clearSetFirst(" - Not available for this state. - ")
        }
    }
}

//Creates the list 
function updateState(state){
    clearSetFirst("- Select a County -");
    for(var i = 0; i < state.length; i++) {
        var opt = state[i];
        //These CANNOT be a reused array, options must be unique, so they are redecalred each iteration.
        var el1 = document.createElement("option");
        el1.textContent = opt;
        el1.value = opt;
        var el2 = document.createElement("option");
        el2.textContent = opt;
        el2.value = opt;
        var el3 = document.createElement("option");
        el3.textContent = opt;
        el3.value = opt;
        var el4 = document.createElement("option");
        el4.textContent = opt;
        el4.value = opt;
        var el5 = document.createElement("option");
        el5.textContent = opt;
        el5.value = opt;
        var el6 = document.createElement("option");
        el6.textContent = opt;
        el6.value = opt;
        var el7 = document.createElement("option");
        el7.textContent = opt;
        el7.value = opt;
        var el8 = document.createElement("option");
        el8.textContent = opt;
        el8.value = opt;
        //I thought about 
        countyDrops[0].appendChild(el1);
        countyDrops[1].appendChild(el2);
        countyDrops[2].appendChild(el3);
        countyDrops[3].appendChild(el4);
        countyDrops[4].appendChild(el5);
        countyDrops[5].appendChild(el6);
        countyDrops[6].appendChild(el7);
        countyDrops[7].appendChild(el8);
    }
}

// Clears the list if no value is selected and sets the first options text to msg 
function clearSetFirst(msg){
    for (var i=0;i<8;i++){
        if (countyDrops[i].value == "") {
            while (countyDrops[i].firstChild) {
                countyDrops[i].removeChild(countyDrops[i].firstChild);
            }
            defaultOpts[i].textContent = msg;
            defaultOpts[i].value = "";
            countyDrops[i].appendChild(defaultOpts[i]);
        }
    }
}

Thank you all for your support. I am definitely open to suggestions for further optimizations, as the code is still 31 KB due to the size of the arrays.

asked	2 months ago
viewed	86 times
active	2 months ago

current community

your communities

more stack exchange communities

Converting repetative javascript statements to arrays in loops

2 Answers

Your Answer

Not the answer you're looking for? Browse other questions tagged javascript arrays out-of-memory infinite-loop or ask your own question.

Hot Network Questions

current community

your communities

more stack exchange communities

Converting repetative javascript statements to arrays in loops

2 Answers

Did you find this question interesting? Try our newsletter

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged javascript arrays out-of-memory infinite-loop or ask your own question.

Related

Hot Network Questions