1

I've been trying to make a tic-tac-toe game but for some reason, the conditional isn't working..

I tried to target the first three elements on the first row and if they all were the same color, I wanted to browser to alert a "win" pop-up, but it doesn't happen.

Does anyone have an idea why?

Here's my code:

var one = document.getElementById("one");
var two = document.getElementById("two");
var three = document.getElementById("three");
var four = document.getElementById("four");
var five = document.getElementById("five");
var six = document.getElementById("six");
var seven = document.getElementById("seven");
var eight = document.getElementById("eight");
var nine = document.getElementById("nine");


var counter = 1;

//code that changes the box color
one.addEventListener("click", function() {
  if (counter < 10 && counter % 2 === 0) {
    one.style.backgroundColor = "yellow";
  } else if (counter < 10 && counter % 2 != 0) {
    one.style.backgroundColor = "red";
  }
  counter++;
  console.log(counter);
});
two.addEventListener("click", function() {
  if (counter < 10 && counter % 2 === 0) {
    two.style.backgroundColor = "yellow";
  } else if (counter < 10 && counter % 2 != 0) {
    two.style.backgroundColor = "red";
  }
  counter++;
  console.log(counter);
});
three.addEventListener("click", function() {
  if (counter < 10 && counter % 2 === 0) {
    three.style.backgroundColor = "yellow";
  } else if (counter < 10 && counter % 2 != 0) {
    three.style.backgroundColor = "red";
  }
  counter++;
  console.log(counter);
});
four.addEventListener("click", function() {
  if (counter < 10 && counter % 2 === 0) {
    four.style.backgroundColor = "yellow";
  } else if (counter < 10 && counter % 2 != 0) {
    four.style.backgroundColor = "red";
  }
  counter++;
  console.log(counter);
});
five.addEventListener("click", function() {
  if (counter < 10 && counter % 2 === 0) {
    five.style.backgroundColor = "yellow";
  } else if (counter < 10 && counter % 2 != 0) {
    five.style.backgroundColor = "red";
  }
  counter++;
  console.log(counter);
});
six.addEventListener("click", function() {
  if (counter < 10 && counter % 2 === 0) {
    six.style.backgroundColor = "yellow";
  } else if (counter < 10 && counter % 2 != 0) {
    six.style.backgroundColor = "red";
  }
  counter++;
  console.log(counter);
});
seven.addEventListener("click", function() {
  if (counter < 10 && counter % 2 === 0) {
    seven.style.backgroundColor = "yellow";
  } else if (counter < 10 && counter % 2 != 0) {
    seven.style.backgroundColor = "red";
  }
  counter++;
  console.log(counter);
});
eight.addEventListener("click", function() {
  if (counter < 10 && counter % 2 === 0) {
    eight.style.backgroundColor = "yellow";
  } else if (counter < 10 && counter % 2 != 0) {
    eight.style.backgroundColor = "red";
  }
  counter++;
  console.log(counter);
});
nine.addEventListener("click", function() {
  if (counter < 10 && counter % 2 === 0) {
    nine.style.backgroundColor = "yellow";
  } else if (counter < 10 && counter % 2 != 0) {
    nine.style.backgroundColor = "red";
  }
  counter++;
  console.log(counter);
});

//logic for winning
if (one.style.backgroundColor == "red" && two.style.backgroundColor == "red" && three.style.backgroundColor == "red") {
  console.log("red wins");
}
.knobs {
  background-color: blue;
  border: none;
  padding: 50px;
  margin: 10px;
}

.knobs:focus {
  outline: none;
}

#total {
  text-align: center;
}
<!DOCTYPE html>

<div id="total">
  <button id="one" class="knobs"></button>
  <button id="two" class="knobs"></button>
  <button id="three" class="knobs"></button>
  <br>
  <button id="four" class="knobs"></button>
  <button id="five" class="knobs"></button>
  <button id="six" class="knobs"></button>
  <br>
  <button id="seven" class="knobs"></button>
  <button id="eight" class="knobs"></button>
  <button id="nine" class="knobs"></button>
</div>

Your help will be much appreciated!

Zze
  • 18,229
  • 13
  • 85
  • 118
Ashish Dommety
  • 152
  • 2
  • 13
  • 2
    Another thing: learn about [event delegation](http://stackoverflow.com/q/1687296/4642212). You don’t need nine distinct event listeners. You only need to bind a single one with an argument (e.g. `e`) to `#total` and check with `e.target` which knob has been clicked. – Sebastian Simon Apr 24 '17 at 01:07
  • I add an alternative solution to your version and a little optimizations – Teocci Apr 24 '17 at 03:17

2 Answers2

3

The problem is that your if statement only runs once in the beginning. You need to turn it into a function and then call it after each event. Something like

function check() {
    if (one.style.backgroundColor == "red" && two.style.backgroundColor == "red" && three.style.backgroundColor == "red"){
    console.log("red wins");
    }
}

At the end of the event listener you would call it like so:

check();

(Like Xufox said above, you should really look into delegation so that this is more manageable and you don't have to repeat everything.)

Pango
  • 663
  • 5
  • 10
0

In your code:

var one = document.getElementById("one");
var two = document.getElementById("two");
var three = document.getElementById("three");
var four = document.getElementById("four");
var five = document.getElementById("five");
var six = document.getElementById("six");
var seven = document.getElementById("seven");
var eight = document.getElementById("eight");
var nine = document.getElementById("nine");

  1. You don't need to declare each of your buttons like this. You can optimize it using an array and a loop like for or forEach. Like this:

    var knobs = [
    'one', 
    'two', 
    'three', 
    'four', 
    'five', 
    'six', 
    'seven', 
    'eight', 
    'nine'
];

knobs.forEach(function(id) {
  var element = document.getElementById(id);
  element.addEventListener('click', function() {
      ...
  }
}

  2. You are adding a listener to each button with addEventListener. This can be optimized as I mention in my point number 1. You get the button element with var element = document.getElementById(id); and the you cand add the listener in the same forEach.

  3. I also used the same forEach to initialize my board array with -1 for each element. This mean it is empty.
  4. Now you need to add a method to verify is there is a winner after each click. In this code sample I used verifyWinner(index); and I passed the index of the button;
  5. Also, in this line one.style.backgroundColor == 'red' only == is not enough so I suggest you to use === for compare strings.
  6. The problem to use this method based on CSS is that what happens if you click two or more times in the same knob? The counter will increase and the knob will change the color from red to yellow for example. That is why I recommend you to use a board array to map the "game status".
  7. Finally, I created a board as an array and I put 1 if is red and 2 if is yellow. This will help you to simplify the verification process.

Here is the code sample. Happy coding. Note: I reduced the knob size for better visualization in the snippet runner. 

var knobs = ['one', 'two', 'three', 'four', 'five', 'six', 'seven', 'eight', 'nine'];
var board = [];

var isRed = true;
var isEndGame = false;
var counter = 0;
knobs.forEach(function(id) {
  var element = document.getElementById(id);
  board.push(-1);
  element.addEventListener('click', function() {
    var index = knobs.indexOf(id);
    if (!isEndGame && board[index] == -1) {
      if (isRed) {
        element.style.backgroundColor = 'red';
        board[index] = 1;
      } else {
        element.style.backgroundColor = 'yellow';
        board[index] = 2;
      }
      verifyWinner(index);
      isRed = !isRed;
      counter++;
      //console.log(counter)
    }
    if (counter == 9) {
      isEndGame = true;
      console.log('End of game.');
    }
  });
});

function verifyWinner(index) {
  //logic for winning
  var player = isRed ? 1 : 2;

  switch (index) {
    case 0:
    case 2:
    case 6:
    case 8:
    case 4:
      verifyVertial(index);
      verifyHorizontal(index);
      verifyDiagonal(index);
      break;
    case 1:
    case 3:
    case 5:
    case 7:
      verifyVertial(index);
      verifyHorizontal(index);
      break;
  }
  if (isEndGame) {
    console.log((isRed ? 'Red' : 'Yellow') + ' has won.');
    console.log('End of game.');
  }
}

function verifyVertial(index) {
  //logic for winning
  if (!isEndGame) {
    var player = isRed ? 1 : 2;
    switch (index) {
      case 0:
      case 3:
      case 6:
        if (board[0] == player && board[3] == player && board[6] == player) {
          isEndGame = true;
        }
        break;
      case 1:
      case 4:
      case 7:
        if (board[1] == player && board[4] == player && board[7] == player) {
          isEndGame = true;
        }
        break;
      case 2:
      case 5:
      case 8: // edges
        if (board[2] == player && board[5] == player && board[8] == player) {
          isEndGame = true;
        }
        break;
    }
  }
}

function verifyHorizontal(index) {
  //logic for winning
  if (!isEndGame) {
    var player = isRed ? 1 : 2;
    switch (index) {
      case 0:
      case 1:
      case 2: // edges
        if (board[0] == player && board[1] == player && board[2] == player) {
          isEndGame = true;
        }
        break;
      case 3:
      case 4:
      case 5:
        if (board[3] == player && board[4] == player && board[5] == player) {
          isEndGame = true;
        }
        break;
      case 6:
      case 7:
      case 8: // edges
        if (board[6] == player && board[7] == player && board[8] == player) {
          isEndGame = true;
        }
        break;
    }
  }
}

function verifyDiagonal(index) {
  //logic for winning
  if (!isEndGame) {
    var player = isRed ? 1 : 2;
    switch (index) {
      case 0:
      case 4:
      case 8: // edges
        if (board[0] == player && board[4] == player && board[8] == player) {
          isEndGame = true;
        }
        break;
      case 2:
      case 4:
      case 6: // edges
        if (board[2] == player && board[4] == player && board[6] == player) {
          isEndGame = true;
        }
        break;
    }
  }
}
.knobs {
  background-color: blue;
  border: none;
  padding: 25px;
  margin: 3px;
}

.knobs:focus .clear:focus {
  outline: none;
}

#total {
  text-align: center;
}

.clear {
  clear: both;
}
<!DOCTYPE html>

<div id="total">
  <button id="one" class="knobs"></button>
  <button id="two" class="knobs"></button>
  <button id="three" class="knobs"></button>
  <div class="clearfix"></div>
  <button id="four" class="knobs"></button>
  <button id="five" class="knobs"></button>
  <button id="six" class="knobs"></button>
  <div class="clearfix"></div>
  <button id="seven" class="knobs"></button>
  <button id="eight" class="knobs"></button>
  <button id="nine" class="knobs"></button>
</div>
Teocci
  • 7,189
  • 1
  • 50
  • 48