Is there a better way to do this?

ditchfieldcaleb

12-29-2010, 02:46 AM

Hey guys. I'm relatively new to Javascript, and wrote a little program. When you click on the change color button, it invokes the function that changes the text/background color. When the text/background colors change, the "teamcolorcheck()" function checks to see if the colors match a certain team, and then respond with a message. I was wondering if there was a way to make this code smaller, more efficient?

<html>

<head>
<script type="text/javascript">
var textcolor = "black"
var color = "white"
var YellowJackets = ["yellow","black","Congratulations! You have picked the awesome team colors of the Yellow Jackets."]
var Dogs = ["red","black","Congratulations! You have picked the awesome team colors of the Georgia Bulldogs."]
var Tigers = ["orange","blue","Yuck! WHY did you pick the colors of such a horrible team?????"]

function teamcolorcheck()
{

if ( (color == YellowJackets[0] && textcolor == YellowJackets[1]) || (textcolor == YellowJackets[0] && color == YellowJackets[1]) )
{ alert(YellowJackets[2]) }

if ( (color == Dogs[0] && textcolor == Dogs[1]) || (textcolor == Dogs[0] && color == Dogs[1]) )
{ alert(Dogs[2]) }

if ( (color == Tigers[0] && textcolor == Tigers[1]) || (textcolor == Tigers[0] && color == Tigers[1]) )
{ alert(Tigers[2]) }

}

function changetextcolor()
{
textcolor = prompt("What color should the text be?","")
document.getElementById('header').style.color = textcolor
teamcolorcheck()
}

function changeBGColor()
{
color = prompt("What color should the background be?","")
document.body.style.backgroundColor = color;
teamcolorcheck()
}

</script>
</head>

<body>
<p id="header"> This will change color if you want it to. YAY!</p>

</script>

<input type="button" value="Click me to change the text color" onclick="changetextcolor()" />
<input type="button" value="Click ME to change the background color" onclick="changeBGColor()" />

</body>

</html>

Old Pedant

12-29-2010, 02:59 AM

ditchfieldcaleb

12-29-2010, 03:04 AM

I'm a bit confused . . . could you explain what the for loop does? I understand for loops, but maybe you could go line by line and explain how it works.

Old Pedant

12-29-2010, 05:45 AM

Maybe you missed this line?

var teams = [ YellowJackets, Dogs, Tigers ];

I took your three arrays and made an uber-array where each of them is simply one element in the teams array.

And then the code is simple:

function teamcolorcheck()
{
// I loop through each element in the teams array...
for ( var t == 0; t < teams.length; ++t )
{
// I pull out one of the elements of the teams array into the variable team
var team = teams[t];
// and then the rest of the code is really yours....just working with, if you will,
// an "alias" of one of your original arrays. That is, the variable team will first
// be that same array as YellowJackets, then the same as Dogs, then Tigers
if ( (color == team[0] && textcolor == team[1])
|| (textcolor == team[0] && color == team[1])
) {
alert( team[2] );
// and the only difference is that as soon as I get a match and display
// the message, I exit the function:
return;
}
}
}

Note that this code would work for any number of teams. Just add them to the end of the teams array and no other changes needed.

ditchfieldcaleb

12-29-2010, 08:07 PM

Thanks man! This was a big help.