HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Style-changing handler for an HTML drop-down box

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
dropstyledownforchanginghandlerboxhtml

Problem

How can I maybe do some loop that will compress the amount of JavaScript/jQuery I need to use. I have a function s3episodesChange() linked to a ` tag. This tag has a few different selectable values that change style properties of various different tags.

All it is doing is showing the appropriate button and making sure all other buttons are hidden well before showing a new one. It also makes sure that when you load the page it will show the first button tied to the first select tags value.

Have a look at my code and tell me what steps I could possibly take to try and achieve this using less overall code.

``
function season3episodesChange() {

//Episodes:
var episode1 = "1 - The Thin White Line";
var episode2 = "2 - Brian Does Hollywood";
var episode3 = "3 - Mr. Griffin Goes to Washington";
var episode4 = "4 - One If by Clam, Two If by Sea";
var episode5 = "5 - And the Wiener Is...";
var episode6 = "6 - Death Lives";
var episode7 = "7 - Lethal Weapons";
-------------------/\---------------------------
//There are 22 total vars just I cut most of them out.

var selectseason3episode = document.getElementById('selectseason3episode');
var season3episode1 = document.getElementById('season3episode1');

if(selectseason3episode.value == episode1){
season3episode1.style.display = 'inline-block';
} else { document.getElementById('season1episode1').style.display = 'none'; }

if(selectseason3episode.value == episode2){
season3episode1.style.display = 'none';
document.getElementById('season3episode2').style.display = 'inline-block';
} else { document.getElementById('season3episode2').style.display = 'none'; }

if(selectseason3episode.value == episode3){
season3episode1.style.display = 'none';
document.getElementById('season3episode3').style.display = 'inline-block';
} else { document.getElementById('season3episode3').style.display = 'none'; }

if(selectseason3episode.value == episode4){
season3episode1.style.display = 'none';
document.getEl

Solution

Instead of having individual variables for each episode, and then iterating over all of them, use an array.

By using an array, you can use a for loop to iterate over all of them, turning the potentially massive amount of loops into one loop:

function season3episodesChange() {

    //Episodes:
    var episodes = [
        "1 - The Thin White Line",
        "2 - Brian Does Hollywood",
        "3 - Mr. Griffin Goes to Washington",
        "4 - One If by Clam, Two If by Sea",
        "5 - And the Wiener Is...",
        "6 - Death Lives",
        "7 - Lethal Weapons"
    ];

    var selectseason3episode = document.getElementById('selectseason3episode');

    for (var i = 1; i <= episodes.length; i++){
        if (selectseason3episode.value == episodes[i - 1]){
            document.getElementById('season3episode'+ i).style.display = 'inline-block';
        } else {
            document.getElementById(['season3episode' + i].join('')).style.display = 'none';
        }
    }
}

Code Snippets

function season3episodesChange() {

    //Episodes:
    var episodes = [
        "1 - The Thin White Line",
        "2 - Brian Does Hollywood",
        "3 - Mr. Griffin Goes to Washington",
        "4 - One If by Clam, Two If by Sea",
        "5 - And the Wiener Is...",
        "6 - Death Lives",
        "7 - Lethal Weapons"
    ];

    var selectseason3episode = document.getElementById('selectseason3episode');

    for (var i = 1; i <= episodes.length; i++){
        if (selectseason3episode.value == episodes[i - 1]){
            document.getElementById('season3episode'+ i).style.display = 'inline-block';
        } else {
            document.getElementById(['season3episode' + i].join('')).style.display = 'none';
        }
    }
}

Context

StackExchange Code Review Q#104277, answer score: 3

Revisions (0)

No revisions yet.