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
    \$\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 May 23 '13 at 9:09
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\$
  • \$\begingroup\$ That's almost perfect but I need also default state. Which is explained in default case. \$\endgroup\$ – Kyrbi May 23 '13 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 May 23 '13 at 13:10
  • \$\begingroup\$ added a check in it! \$\endgroup\$ – Topener May 23 '13 at 13:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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