Jbur43
Jbur43

Reputation: 1312

Refactoring long if/else if statement with for loop - javascript

I know there is a way for me to refactor this code, but I can't figure it out and I could use some help. I created a tic tac toe game and created the functionality to declare a winner, it works, but it's way too long.

$(document).ready(function() {

    var turn = 0;
    // var winOptions = [['#1','#2','#3'], ['#4','#5','#6'], ['#7','#8','#9'], [1,5,9], [3,5,7], [1,4,7], [3,6,9], [2,4,8]];

    $('td').on("click", function() {
        if (turn % 2 === 0) {
            $(this).text("0");
        } else {
            $(this).text("X");
        }
        $(this).off("click");
        turn++;
        checkForWinner()
    });


    function checkForWinner() {
        if ($('#1').text()==='X' && $('#2').text()==='X' && $('#3').text()==='X')
            alert('you win!');
        else if ($('#4').text()==='X' && $('#5').text()==='X' && $('#6').text()==='X')
            alert('you win!');
        else if ($('#7').text()==='X' && $('#8').text()==='X' && $('#9').text()==='X')
            alert('you win!');
        else if ($('#1').text()==='X' && $('#5').text()==='X' && $('#9').text()==='X')
            alert('you win!');
        else if ($('#3').text()==='X' && $('#5').text()==='X' && $('#7').text()==='X')
            alert('you win!');
        else if ($('#1').text()==='X' && $('#4').text()==='X' && $('#7').text()==='X')
            alert('you win!');
        else if ($('#3').text()==='X' && $('#6').text()==='X' && $('#9').text()==='X')
            alert('you win!');
        else if ($('#2').text()==='X' && $('#4').text()==='X' && $('#8').text()==='X')
            alert('you win!');
        else if ($('#1').text()==='O' && $('#2').text()==='O' && $('#3').text()==='O')
            alert('you win!');
        else if ($('#4').text()==='O' && $('#5').text()==='O' && $('#6').text()==='O')
            alert('you win!');
        else if ($('#7').text()==='O' && $('#8').text()==='O' && $('#9').text()==='O')
            alert('you win!');
        else if ($('#1').text()==='O' && $('#5').text()==='O' && $('#9').text()==='O')
            alert('you win!');
        else if ($('#3').text()==='O' && $('#5').text()==='O' && $('#7').text()==='O')
            alert('you win!');
        else if ($('#1').text()==='O' && $('#4').text()==='O' && $('#7').text()==='O')
            alert('you win!');
        else if ($('#3').text()==='O' && $('#6').text()==='O' && $('#9').text()==='O')
            alert('you win!');
        else if ($('#2').text()==='O' && $('#4').text()==='O' && $('#8').text()==='O')
            alert('you win!');
        }
});

I started making the array variable win options to loop through, but it still isn't working.

FYI, those selectors are the id's for each td tag in my HTML.

Thanks for taking a look at this.

Upvotes: 1

Views: 862

Answers (1)

Ben Cook
Ben Cook

Reputation: 1684

Some refactoring along the lines of Shilly's comment might be called for. However, without drastically changing your design, you could do something like the following:

var winConditions = [
   ['#1','#2','#3'], ['#4','#5','#6'], ['#7','#8','#9'], // rows
   ['#1','#4','#7'], ['#2','#5','#8'], ['#3','#6','#9'], // columns
   ['#1','#5','#9'], ['#3','#5','#7']                    // diagonals
];

for (var i = 0, len = winConditions.length; i < len; i++) {
  var text = $(winConditions[i][0]).text();
  if (
    (text === 'X' || text === 'O') && 
    $(winConditions[i][1]).text() === text && $(winConditions[i][2]).text() === text
  ) {
    alert('you win!');
    break;
  }
}

Upvotes: 1

Related Questions