# Checking winning conditions in Tic-Tac-Toe

This code checks winning conditions in Tic-Tac-Toe by checking if there is any row, column or diagonal with the same symbols.

The board is a 2-dimensional array of chars. The character ' ' means that a field is empty.

How can I refactor/simplify this code?

 public static bool SomeoneWins(char[][] board)
{
// Check columns
for (var x = 0; x < board.Length; x++)
{
var firstField = board[x][0];
if (firstField == ' ') continue;
bool allFieldsTheSame = true;

for (var y = 1; y < board[x].Length; y++)
{
if (board[x][y] != firstField)
{
allFieldsTheSame = false;
break;
}
}

if (allFieldsTheSame) return true;
}

// Check rows
for (var y = 0; y < board.Length; y++)
{
var firstField = board[0][y];
if (firstField == ' ') continue;
var allFieldsTheSame = true;

for (var x = 1; y < board.Length; x++)
{
if (board[x][y] != firstField)
{
allFieldsTheSame = false;
break;
}
}

if (allFieldsTheSame) return true;
}

// first diagonal
if (board[0][0] != ' ')
{
var allFieldsTheSame = true;

for (var d = 0; d < board.Length; d++)
{
if (board[d][d] != board[0][0])
{
allFieldsTheSame = false;
break;
}
}
if (allFieldsTheSame) return true;
}

// second diagonal
if ( board[board.Length - 1][0] != ' ')
{
var allFieldsTheSame = true;
for (var d = 0; d < board.Length; d++)
{
if (board[d][board.Length-d-1] !=  board[board.Length - 1][0])
{
allFieldsTheSame = false;
break;
}
}
if (allFieldsTheSame) return true;
}

return false;
}

• Have you actually tried this code? There seem to be at least one obvious mistake in for (var x = 1; y < board.Length; x++) Commented Jul 18, 2015 at 14:56
• @barakmanos Yes and this is not a mistake. Note that zeroth element is checked before the loop.
– Kao
Commented Jul 20, 2015 at 7:57
• I am not referring to the fact that you start from the first element, but to the fact that you increment x while checking y < length. Commented Jul 20, 2015 at 8:11
• Oh, now I see. You are right, of course.
– Kao
Commented Jul 20, 2015 at 8:54

What does all winning conditions in Tic Tac Toe have in common? They are all straight lines!

The idea of having one method that can be called multiple times is a good one, to do that we need to input the starting position, and how much we should change x and y with every time.

We also need a way to stop the loop, we can either stop when we notice that we will go out of bounds, or we can stop after a specific number of checks. In this implementation, I chose to hard-code 3 as the limit for how many tiles to check.

public static bool AllFieldsTheSame(int startX, int startY, char[][] board, int dx, int dy)
{
char firstField = board[startY][startX]
if (firstField == ' ')
{
return false;
}

for (var i = 0; i < 3; i++)
{
int y = startY + dy * i;
int x = startX + dx * i;
if (board[y][x] != firstField)
{
return false;
}
}

return true;
}


Then this method can be called repeatedly as follows:

public static bool SomeoneWins(char[][] board)
{
// Check columns
for (var x = 0; x < board.Length; x++)
{
if (AllFieldsTheSame(x, 0, board, 0, 1))
return true;
}

// Check rows
for (var y = 0; y < board.Length; y++)
if (AllFieldsTheSame(0, y, board, 1, 0))
return true;

// Check diagonals
if (AllFieldsTheSame(0, 0, board, 1, 1))
return true;

if (AllFieldsTheSame(2, 0, board, -1, 1))
return true;
}


However, I suspect you are also interested in who wins, in which case you could have both AllFieldsTheSame and SomeoneWins return a char instead of a bool.

And by the way, I'd prefer to use an enum for the possible values of each tile. A char can have the value of Q, but I don't believe you want to place a Q tile in your game. Using an enum reduces the eliminates any possible risk of invalid characters.

• Mixing bracing style (with/without braces) should be avoided. Commented Jul 20, 2015 at 6:07
• @Heslacher Yes, it should. I hope the OP does that when using this code. Commented Jul 20, 2015 at 10:19

There seem to be at least one obvious mistake in for (var x = 1; y < board.Length; x++).

In addition to that, you may as well generalize the code for checking rows and columns:

public static bool AllFieldsTheSame(char[][] board, char firstField, int m)
{
if (firstField == ' ')
return false;

for (var n = 1; n < board[m].Length; n++)
if (board[m][n] != firstField)
return false;

return true;
}

public static bool SomeoneWins(char[][] board)
{
// Check columns
for (var x = 0; x < board.Length; x++)
if (AllFieldsTheSame(board, board[x][0], x)
return true;

// Check rows
for (var y = 0; y < board.Length; y++)
if (AllFieldsTheSame(board, board[0][y], y);
return true;

// The rest of your code below...
}

• I don't believe this refactoring will work. Are you sure about if (board[m][n] != firstField) ? Commented Jul 19, 2015 at 12:39
• It is impossible for your AllFieldsTheSame method to check the win conditions on both columns and rows. Commented Jul 20, 2015 at 10:20