HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Command Prompt TicTacToe in C#

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
commandtictactoeprompt

Problem

I wrote a basic command prompt Tic Tac Toe Game. I want to know what can be improved in terms of modeling and what mistakes I've made (if any).

void Main()
{
    TicTacToeBoard board = new TicTacToeBoard();

    while(!board.GameOver())
    {
        var i = int.Parse(Console.ReadLine());
        board.MakeMove(i);
        Console.WriteLine(board.DrawBoard());
    }   
}

class TicTacToeBoard
{
    SquareType[] board = new SquareType[9];

    SquareType currentType = SquareType.X;

    public void MakeMove(int i)
    {
        if(board[i] != SquareType.Empty) return;
            board[i] = currentType;
            currentType = OppositeType(currentType);
    }

    SquareType OppositeType(SquareType squareType) => 
        squareType == SquareType.X ? SquareType.O : SquareType.X;

    public bool GameOver()
    {
        return WeHaveWinner() || WeHaveDraw();
    }

    bool WeHaveWinner()
    {
        var winningIndecies = new[]
        {
            new[]{1,2,3},
            new[]{4,5,6},
            new[]{7,8,9},
            new[]{1,5,9},
            new[]{3,5,7}
        };

        foreach(var indecies in winningIndecies)
        {
            var squares = indecies.Select(i => board[i - 1]);
            if(squares.Any(x => x == SquareType.Empty)) continue;
            else if(squares.Distinct().Count() == 1) return true;
        }
        return false;
    }

    bool WeHaveDraw() => !WeHaveWinner() && board.All(x => x != SquareType.Empty);

    public string DrawBoard()
        => $@"|{board[0]},{board[1]},{board[2]}|
|{board[3]},{board[4]},{board[5]}|
|{board[6]},{board[7]},{board[8]}|".Replace("Empty","  ");
}

enum SquareType { Empty, X, O }

Solution

Overall it looks good to me.
A few points though:

  • int.Parse will throw if Console.ReadLine returns a non integer. Perhaps an int.TryParse and display some help (position: 0 - 8)



  • It's a small app but since for the whole lifetime of your application, in each iteration you'll call WeHaveWinner. I'd initialize that array as a field to that class on object construction.



  • On WeHaveWinner you'll (possibly) be enumerating the return of .Select() twice. Consider calling ToList() on it.



  • The main method, could be:



do
        {
            var i = int.Parse(Console.ReadLine());
            board.MakeMove(i);
            Console.WriteLine(board.DrawBoard());
        } while (!board.GameOver());


This way you won't be checking for winnings before the first input

  • Your string replace at the end on Empty could lead to a issue if the Enum Empty was renamed. Consider replacing (SquareType.Empty, " ")



  • You could consider replacing the Enum with a class, that would give you ability to use polymorphism. There you could override ToString to display (X or Y or empty) which would help you get rid of that String.Replace altogether.


Something along the lines of: https://stackoverflow.com/questions/1376312/whats-the-equivalent-of-javas-enum-in-c

+1 for the verbatin + interpolation

Code Snippets

do
        {
            var i = int.Parse(Console.ReadLine());
            board.MakeMove(i);
            Console.WriteLine(board.DrawBoard());
        } while (!board.GameOver());

Context

StackExchange Code Review Q#119178, answer score: 5

Revisions (0)

No revisions yet.