CODEWITHSUNDEEP
javascript
XPOLKYT STUDIOS
XPOLKYT STUDIOS

Reputation: 3

How to make this js repetitive code into a loop?

I have this code, repeating 20 times with only change of variables prefix. How can i possibly make a loop and iterate by it to avoid this huge block of code? It is used in my website and I want to make the development process more clear.

var keepElements = document.getElementsByName("keep-type");
for(var i = 0; i < keepElements.length; i++){
    keep = document.getElementById(keepElements[i].value);
    if(keepElements[i].checked == true){
        keep.style.display = "block";
    }
    else{keep.style.display = "none";}
}

var offworkshopElements = document.getElementsByName("offworkshop-type");
for(var i = 0; i < offworkshopElements.length; i++){
    offworkshop = document.getElementById(offworkshopElements[i].value);
    if(offworkshopElements[i].checked == true){
        offworkshop.style.display = "block";
    }
    else{offworkshop.style.display = "none";}
}

var defworkshopElements = document.getElementsByName("defworkshop-type");
for(var i = 0; i < defworkshopElements.length; i++){
    defworkshop = document.getElementById(defworkshopElements[i].value);
    if(defworkshopElements[i].checked == true){
        defworkshop.style.display = "block";
    }
    else{defworkshop.style.display = "none";}
}

Upvotes: 0

Views: 106

Answers (3)

Michael Beeson
Michael Beeson

Reputation: 2875

The answers saying you should put the common functionality into a single function are absolutely right.

I just wanted to add that there are also solutions on the query side of things:

you could have all the elements be collected in one javascript function, querySelectorAll()

const elements = document.querySelectorAll('[name="keep-type"], [name="offworkshop-type"], [name="defworkshop-type"]')

Alternatively you could give them all the same class and select by that class.

const elements = document.getElementsByClassName("requiredElements");

And this way you have all your elements in one array.

Upvotes: 1

jabaa
jabaa

Reputation: 6727

If I didn't miss any details, you're repeating the same block 3 times:

["keep-type", "offworkshop-type", "defworkshop-type"].forEach(name => {
    var elements = document.getElementsByName(name);
    for(var i = 0; i < elements.length; i++){
        element = document.getElementById(elements[i].value);
        if(elements[i].checked == true){
            element.style.display = "block";
        }
        else{element.style.display = "none";}
    }
});

I prefer the array / forEach syntax, because the block starts with the list of name. But you could use a for...of loop instead.

for (const name of ["keep-type", "offworkshop-type", "defworkshop-type"]) {
    var elements = document.getElementsByName(name);
    for(var i = 0; i < elements.length; i++){
        element = document.getElementById(elements[i].value);
        if(elements[i].checked == true){
            element.style.display = "block";
        }
        else{element.style.display = "none";}
    }
}

Upvotes: 1

JClarkCDS
JClarkCDS

Reputation: 419

Move the logic to a function:

function keepElements(name) {
    var keepElements = document.getElementsByName(name);
    for (var i = 0; i < keepElements.length; i++) {
        keep = document.getElementById(keepElements[i].value);
        if (keepElements[i].checked == true) {
            keep.style.display = "block";
        }
        else { keep.style.display = "none"; }
    }
}

keepElements("keep-type");
keepElements("offworkshop-type");
keepElements("defworkshop-type");

Upvotes: 2

Related Questions