C# Coding a Tie in Tic Tac Toe

Question

I've developed a game of tic tac toe in C# and set a function for the win conditions, but I can't seem to figure out how to set the condition for a tie when the grid is full. When I tried to input, the tie would occur randomly. I wonder if it's how I set the or and and operators.Here's what I tried.

public bool Ended() 
{
    if (grid[0, 0] == 'X' && grid[0, 1] == 'X' && grid[0, 2] == 'X') {  Console.WriteLine("PLayer 1 Wins"); return true; }
    if (grid[1, 0] == 'X' && grid[1, 1] == 'X' && grid[1, 2] == 'X') {  Console.WriteLine("PLayer 1 Wins"); return true; }
    if (grid[2, 0] == 'X' && grid[2, 1] == 'X' && grid[2, 2] == 'X') {  Console.WriteLine("PLayer 1 Wins"); return true; }

    if (grid[0, 0] == 'X' && grid[1, 0] == 'X' && grid[2, 0] == 'X') {  Console.WriteLine("PLayer 1 Wins"); return true; }
    if (grid[0, 1] == 'X' && grid[1, 1] == 'X' && grid[2, 1] == 'X') {  Console.WriteLine("PLayer 1 Wins"); return true; }
    if (grid[0, 2] == 'X' && grid[1, 2] == 'X' && grid[2, 2] == 'X') {  Console.WriteLine("PLayer 1 Wins"); return true; }

    if (grid[0, 0] == 'X' && grid[1, 1] == 'X' && grid[2, 2] == 'X') {  Console.WriteLine("PLayer 1 Wins"); return true; }
    if (grid[0, 2] == 'X' && grid[1, 1] == 'X' && grid[2, 0] == 'X') {  Console.WriteLine("PLayer 1 Wins"); return true; }

    if (grid[0, 0] == 'O' && grid[0, 1] == 'O' && grid[0, 2] == 'O') { Console.WriteLine("PLayer 2 Wins"); return true; }
    if (grid[1, 0] == 'O' && grid[1, 1] == 'O' && grid[1, 2] == 'O') { Console.WriteLine("PLayer 2 Wins"); return true; }
    if (grid[2, 0] == 'O' && grid[2, 1] == 'O' && grid[2, 2] == 'O') { Console.WriteLine("PLayer 2 Wins"); return true; }

    if (grid[0, 0] == 'O' && grid[1, 0] == 'O' && grid[2, 0] == 'O') { Console.WriteLine("PLayer 2 Wins"); return true; }
    if (grid[0, 1] == 'O' && grid[1, 1] == 'O' && grid[2, 1] == 'O') { Console.WriteLine("PLayer 2 Wins"); return true; }
    if (grid[0, 2] == 'O' && grid[1, 2] == 'O' && grid[2, 2] == 'O') { Console.WriteLine("PLayer 2 Wins"); return true; }

    if (grid[0, 0] == 'O' && grid[1, 1] == 'O' && grid[2, 2] == 'O') { Console.WriteLine("PLayer 2 Wins"); return true; }
    if (grid[0, 2] == 'O' && grid[1, 1] == 'O' && grid[2, 0] == 'O') { Console.WriteLine("PLayer 2 Wins"); return true; }

    else if (grid[0, 0] == 'X' || grid[0, 0] == 'O' &&
        grid[0, 1] == 'X' || grid[0, 1] == 'O' &&
        grid[0, 2] == 'X' || grid[0, 2] == 'O' &&
        grid[1, 0] == 'X' || grid[1, 0] == 'O' &&
        grid[1, 1] == 'X' || grid[1, 1] == 'O' &&
        grid[1, 2] == 'X' || grid[1, 2] == 'O' &&
        grid[2, 0] == 'X' || grid[2, 0] == 'O' &&
        grid[2, 1] == 'X' || grid[2, 1] == 'O' &&
        grid[2, 2] == 'X' || grid[2, 2] == 'O')
    { Console.WriteLine("It's a Tie"); return true; }
    return false;
}

Answer 1

Here's a way to check. First create all 8 slices of the grid:

var slices =
    Enumerable
        .Range(0, 3)
        .SelectMany(i => new []
        {
            Enumerable.Range(0, 3).Select(j => grid[i, j]),
            Enumerable.Range(0, 3).Select(j => grid[j, i])
        })
        .Concat(new [] { Enumerable.Range(0, 3).SelectMany(i => new [] { grid[i, i] }) })
        .Concat(new [] { Enumerable.Range(0, 3).SelectMany(i => new [] { grid[i, 2 - i] }) })
        .Select(z => String.Join("", z))
        .ToArray();

Starting with this input:

char[,] grid = new char[3, 3]
{
    { 'X', 'O', 'X' },
    { 'O', 'X', 'X' },
    { 'X', 'X', 'O' },
};

That'll give you:

Now it's easy to see who wins:

bool xWins = slices.Any(z => z == "XXX") && slices.All(z => z != "OOO");
bool oWins = slices.Any(z => z == "OOO") && slices.All(z => z != "XXX");

And to see if the board is a draw check that both are false and if there are any empty spots (assuming that _ is empty):

bool draw = !xWins && !oWins && slices.All(z => !z.Contains("_"));

Answer 2

The fundamental problem is that the indentation of your program does not match its meaning. Suppose I wrote:

int x =
   2 + 3 * 
   4 + 5;

It would be very easy to think that this should be 5 x 9 = 45, but in fact it is 2 + 12 + 5 = 19.

You've done the same thing.

if (a || b &&
    c || d)

means

if (a || (b && c) || d)

not what you want it to mean, which is

if ((a || b) && (c || d))

You probably already remember that multiplication is higher precedence than addition. You can remember that AND is higher than OR because AND is the same as multiplication!

true && true == true
true && false == false
false && true == false
false && false == false

If we replace true with 1, false with 0 and && with * then we get true statements out:

1 * 1 == 1
1 * 0 == 0
0 * 1 == 0
0 * 0 == 0

Now let's address the other problems in your program.

Any time you find you're writing the same code twice or more, consider making a method. Cutting and pasting is a bad habit in code.

if (grid[0, 0] == 'X' && grid[0, 1] == 'X' && grid[0, 2] == 'X') {  Console.WriteLine("PLayer 1 Wins"); return true; }
if (grid[1, 0] == 'X' && grid[1, 1] == 'X' && grid[1, 2] == 'X') {  Console.WriteLine("PLayer 1 Wins"); return true; }
if (grid[2, 0] == 'X' && grid[2, 1] == 'X' && grid[2, 2] == 'X') {  Console.WriteLine("PLayer 1 Wins"); return true; }

I notice that you've cut and pasted a typo! Just another reason why its so bad.

If you find you're cutting and pasting, make a method:

private bool HasRow(char player, int row)
{
  return grid[row,0] == player && grid[row,1] == player && grid[row,2] == player;
}

And now you can replace that code with

if (HasRow('X', 0) || HasRow('X', 1) || HasRow('X', 2)) { ... }
...
if (HasRow('O', 0) || HasRow('O', 1) || HasRow('O', 2)) { ... }

But wait, we just cut and pasted code again. So fix that!

private bool HasAnyRow(char player) 
{
  return HasRow(player, 0) || HasRow(player, 1) || HasRow(player, 2);
}

And now your code becomes

if (HasAnyRow('X')) { ... }
...
if (HasAnyRow('O')) { ... }
...

Do the same thing for columns and diagonals and you end up with

if (HasAnyRow('X') || HasAnyColumn('X') || HasAnyDiagonal('X')) { ... }
if (HasAnyRow('O') || HasAnyColumn('O') || HasAnyDiagonal('O')) { ... }

But wait, this is still too much copy pasting.

private bool HasWin(char player) 
{
  return HasAnyRow(player) || HasAnyColumn(player) || HasAnyDiagonal(player);
}

And now the code is:

if (HasWin('X')) { ... }
if (HasWin('O')) { ... }

Your code will get much better -- easier to write, easier to read, easier to debug, more likely correct the first time -- if you:

• Eliminate redundant code by making helper methods instead of copy-pasting code.
• Make helper methods that are in the business domain of the program. What is a win in tic-tac-toe? Having a row, column or diagonal. So make methods called HasRow, HasColumn and HasDiagonal, and now you can reason about your program more easily because it is written using the concepts from the business domain of the program.

Answer 3

This solution works around your problem by removing the faulty code altogether and using a for-loop-based approach for the last check. It also avoids redundancy and makes it (imho) easier to follow:

private const char PLAYER_1_SYMBOL = 'X';
private const char PLAYER_2_SYMBOL = 'O';

private readonly char[,] grid = new char[3,3];

public bool Ended()
{
    if (CheckPlayerVictory(PLAYER_1_SYMBOL))
    {
        Console.WriteLine("Player 1 Wins");
        return true;
    }
    else if (CheckPlayerVictory(PLAYER_2_SYMBOL))
    {
        Console.WriteLine("Player 2 Wins");
        return true;
    }
    else if (CheckNoMovesRemaining())
    {
        Console.WriteLine("It's a Tie");
        return true;
    }
    return false;
}

// true, if a match for the given symbol was found in the grid.
private bool CheckPlayerVictory(char symbol)
{
    return (grid[0, 0] == symbol && grid[0, 1] == symbol && grid[0, 2] == symbol)
        || (grid[1, 0] == symbol && grid[1, 1] == symbol && grid[1, 2] == symbol)
        || (grid[2, 0] == symbol && grid[2, 1] == symbol && grid[2, 2] == symbol)
        || (grid[0, 0] == symbol && grid[1, 0] == symbol && grid[2, 0] == symbol)
        || (grid[0, 1] == symbol && grid[1, 1] == symbol && grid[2, 1] == symbol)
        || (grid[0, 2] == symbol && grid[1, 2] == symbol && grid[2, 2] == symbol)
        || (grid[0, 0] == symbol && grid[1, 1] == symbol && grid[2, 2] == symbol)
        || (grid[0, 2] == symbol && grid[1, 1] == symbol && grid[2, 0] == symbol);
}

// true, if no free grid cell is left.
private bool CheckNoMovesRemaining()
{
    int numFreeSpaces = 9;
    for (int i = 0; i < 3; i++)
    {
        for (int j = 0; j < 3; j++)
        {
            if (grid[i,j] == PLAYER_1_SYMBOL || grid[i,j] == PLAYER_2_SYMBOL)
            {
                numFreeSpaces--;
            }
        }
    }

    return numFreeSpaces <= 0;
}

Answer 4

Check if the grid variable does not contain '' (or whatever you use for empty)

Answer 5

|| operator has lower priority than &&. put or conditions in brackets:

if ((grid[0, 0] == 'X' || grid[0, 0] == 'O') &&
    (grid[0, 1] == 'X' || grid[0, 1] == 'O') &&
    (grid[0, 2] == 'X' || grid[0, 2] == 'O') &&
    (grid[1, 0] == 'X' || grid[1, 0] == 'O') &&
    (grid[1, 1] == 'X' || grid[1, 1] == 'O') &&
    (grid[1, 2] == 'X' || grid[1, 2] == 'O') &&
    (grid[2, 0] == 'X' || grid[2, 0] == 'O') &&
    (grid[2, 1] == 'X' || grid[2, 1] == 'O') &&
    (grid[2, 2] == 'X' || grid[2, 2] == 'O'))
{ 
    Console.WriteLine("It's a Tie"); 
    return true; 
}