1
\$\begingroup\$

I made this Javascript to hide, show and change some DIVs but I believe it's not really good code. Can you help me to make it better?

function colorM(n) {
    switch(n){
        case 1:
            document.getElementById("slideM1").style.backgroundColor="#009CFF";
            document.getElementById("slideM1").className="show";
            document.getElementById("slideM2").style.backgroundColor="silver";
            document.getElementById("slideM3").style.backgroundColor="silver";
            document.getElementById("slideM4").style.backgroundColor="silver";
            document.getElementById("slideM5").style.backgroundColor="silver";
            document.getElementById("slideM2").className="";
            document.getElementById("slideM3").className="";
            document.getElementById("slideM4").className="";
            document.getElementById("slideM5").className="";
            document.getElementById("slideC1").style.display="block";
            document.getElementById("slideC2").style.display="none";
            document.getElementById("slideC3").style.display="none";
            document.getElementById("slideC4").style.display="none";
            document.getElementById("slideC5").style.display="none";
            break;
        case 2:
            document.getElementById("slideM2").style.backgroundColor="#009CFF";
            document.getElementById("slideM2").className="show";
            document.getElementById("slideM1").style.backgroundColor="silver";
            document.getElementById("slideM3").style.backgroundColor="silver";
            document.getElementById("slideM4").style.backgroundColor="silver";
            document.getElementById("slideM5").style.backgroundColor="silver";
            document.getElementById("slideM1").className="";
            document.getElementById("slideM3").className="";
            document.getElementById("slideM4").className="";
            document.getElementById("slideM5").className="";
            document.getElementById("slideC1").style.display="none";
            document.getElementById("slideC2").style.display="block";
            document.getElementById("slideC3").style.display="none";
            document.getElementById("slideC4").style.display="none";
            document.getElementById("slideC5").style.display="none";
            break;
        case 3:
            document.getElementById("slideM3").style.backgroundColor="#009CFF";
            document.getElementById("slideM3").className="show";
            document.getElementById("slideM1").style.backgroundColor="silver";
            document.getElementById("slideM2").style.backgroundColor="silver";
            document.getElementById("slideM4").style.backgroundColor="silver";
            document.getElementById("slideM5").style.backgroundColor="silver";
            document.getElementById("slideM1").className="";
            document.getElementById("slideM2").className="";
            document.getElementById("slideM4").className="";
            document.getElementById("slideM5").className="";
            document.getElementById("slideC1").style.display="none";
            document.getElementById("slideC2").style.display="none";
            document.getElementById("slideC3").style.display="block";
            document.getElementById("slideC4").style.display="none";
            document.getElementById("slideC5").style.display="none";
            break;
        case 4:
            document.getElementById("slideM4").style.backgroundColor="#009CFF";
            document.getElementById("slideM4").className="show";
            document.getElementById("slideM1").style.backgroundColor="silver";
            document.getElementById("slideM2").style.backgroundColor="silver";
            document.getElementById("slideM3").style.backgroundColor="silver";
            document.getElementById("slideM5").style.backgroundColor="silver";
            document.getElementById("slideM1").className="";
            document.getElementById("slideM2").className="";
            document.getElementById("slideM3").className="";
            document.getElementById("slideM5").className="";
            document.getElementById("slideC1").style.display="none";
            document.getElementById("slideC2").style.display="none";
            document.getElementById("slideC3").style.display="none";
            document.getElementById("slideC4").style.display="block";
            document.getElementById("slideC5").style.display="none";
            break;
        case 5:
            document.getElementById("slideM5").style.backgroundColor="#009CFF";
            document.getElementById("slideM5").className="show";
            document.getElementById("slideM1").style.backgroundColor="silver";
            document.getElementById("slideM2").style.backgroundColor="silver";
            document.getElementById("slideM3").style.backgroundColor="silver";
            document.getElementById("slideM4").style.backgroundColor="silver";
            document.getElementById("slideM1").className="";
            document.getElementById("slideM2").className="";
            document.getElementById("slideM3").className="";
            document.getElementById("slideM4").className="";
            document.getElementById("slideC1").style.display="none";
            document.getElementById("slideC2").style.display="none";
            document.getElementById("slideC3").style.display="none";
            document.getElementById("slideC4").style.display="none";
            document.getElementById("slideC5").style.display="block";
            break;
        default: 
            document.getElementById("slideM1").style.backgroundColor="#009CFF";
            document.getElementById("slideM1").className="show";
            document.getElementById("slideM2").style.backgroundColor="silver";
            document.getElementById("slideM3").style.backgroundColor="silver";
            document.getElementById("slideM4").style.backgroundColor="silver";
            document.getElementById("slideM5").style.backgroundColor="silver";
            document.getElementById("slideM2").className="";
            document.getElementById("slideM3").className="";
            document.getElementById("slideM4").className="";
            document.getElementById("slideM5").className="";
            document.getElementById("slideC1").style.display="block";
            document.getElementById("slideC2").style.display="none";
            document.getElementById("slideC3").style.display="none";
            document.getElementById("slideC4").style.display="none";
            document.getElementById("slideC5").style.display="none";
            break;
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ For starters: Instead of clearing the className of every div that you want to hide, assign a class name that already has the background-color: 'silver' and display: none. That alone saves you a lot of lines. \$\endgroup\$
    – Amberlamps
    Commented May 23, 2013 at 9:09

1 Answer 1

3
\$\begingroup\$

Loop through the ID's instead of doing a switch. This should do the same as yours

function colorM(id){

    // if not a number
    if (!(!isNaN(parseFloat(n)) && isFinite(n))) id =1;
    // default for numbers
    if (id > 5 || id < 1) id = 1;

    for (var i = 1; i <= 5; i++){

        if (id == i){
            document.getElementById("slideM" + i).style.backgroundColor="#009CFF";
            document.getElementById("slideM" + i).className="show";
            document.getElementById("slideC" + i).style.display="block";
        }
        else {
            document.getElementById("slideM" + i).style.backgroundColor="silver";
            document.getElementById("slideM" + i).className="";
            document.getElementById("slideC" + i).style.display="none";
        }
    }
}

Edit: added a check if id is numeric AND id is within range

\$\endgroup\$
3
  • \$\begingroup\$ That's almost perfect but I need also default state. Which is explained in default case. \$\endgroup\$
    – Kyrbi
    Commented May 23, 2013 at 11:47
  • \$\begingroup\$ The default is the same as 1? So make it in such a way that if you are trying to default it, (it is not any of the 5) it'll give 1 to the function \$\endgroup\$
    – Topener
    Commented May 23, 2013 at 13:10
  • \$\begingroup\$ added a check in it! \$\endgroup\$
    – Topener
    Commented May 23, 2013 at 13:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.