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?

Code:

<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

Code:

<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?????"]

var teams = [ YellowJackets, Dogs, Tigers ];			
			
function teamcolorcheck()
{
    for ( var t == 0; t < teams.length; ++t )
    {
       var team = teams[t];				
       if  (    (color == team[0] && textcolor == team[1]) 
            || (textcolor == team[0] && color == team[1]) 
       ) {  
           alert( team[2] );
           return;
       }
    }
}
...

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?

Code:

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:

Code:

			
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.