Checking winning conditions in Tic-Tac-Toe

Question

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;
        }

Answer 1

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.

Answer 2

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...
}