My design goal was to try and make a clean and well-designed Tic-Tac-Toe game but also to try and use concepts and programming constructs that I'm new to. I'm planning to implement an AI Player class later on.  I know that Tic-Tac-Toe is a solved game so there are more effective approaches for AI logic but I've never programmed an AI before so my idea is to at runtime generate a database of all possible gamestates, and then have the AI rank each intermediate gamestate by the amount of positive outcomes (that is where the AI wins) - negative outcomes when it decides where to place its marker. Would this be a good idea and would it work at all?Both positive and negative feedback is welcome.Game.cspublic partial class Game : Form{    #region Fields & Properties    internal static VisualCell[,] vGrid;    internal static Player[] players;    #endregion    #region Constructors    public Game()    {        InitializeComponent();    }    #endregion    #region Instanced Methods    private void Game_Load(object sender, EventArgs e)    {        players = new Player[2];        players[0] = new Player(); // Set to human player or AI player        players[1] = new Player(); // Always human        GameRound.Complete += ShowCompletionDialog;        GenerateGridBtns();        Task roundTask = Task.Factory.StartNew(() => new GameRound().Start());    }    private void ShowCompletionDialog(OutcomeType gameOutcome)    {        long pID = (long)gameOutcome + 1;        string WonText = Player  + pID +  won!;        if (gameOutcome == OutcomeType.Draw)        { //*HACK*: If the marker is empty it means that the passed player was a dummy and game was a draw            WonText = The game ended in a draw!;        }        DialogResult dialogResult = MessageBox.Show(Play again?, WonText, MessageBoxButtons.YesNo);        if (dialogResult == DialogResult.Yes)        {            RestartGame();            // TODO: Add a bunch of resets here...        } else if (dialogResult == DialogResult.No)        {            //do something else        }    }    private void GenerateGridBtns()    {        vGrid = new VisualCell[3, 3];        for (int x = 0; x < 3; x++)        {            for (int y = 0; y < 3; y++)            {                vGrid[x, y] = new VisualCell()                 {                     AutoSize = true,                     Location = new Point(x*110, y*110),                     Size = new Size(124, 124),                    Enabled = false,                    Font = new Font(Font.FontFamily, 40),                    CellPos = new Tuple<int,int>(x, y)                };                Controls.Add(vGrid[x, y]);            }        }    }    private void RestartGame()     {        for (int x = 0; x < 3; x++)        {            for (int y = 0; y < 3; y++)            {                vGrid[x, y].Invoke(new Action(() => vGrid[x, y].Text = String.Empty));                vGrid[x, y].Invoke(new Action(() => vGrid[x, y].Enabled = false));            }        }        var round = new GameRound();        Task roundTask = Task.Factory.StartNew(() => round.Start());    }    #endregion }VCell.csclass VisualCell : Button{    #region Fields & Properties    public Tuple<int, int> CellPos { get; set; }    protected override bool ShowFocusCues    {        get        {            return false;        }    }    #endregion    #region Constructors    public VisualCell(): base()    {        this.SetStyle(ControlStyles.Selectable, false);    }    #endregion}Cell.csclass Cell{    #region Properties & Fields    private Mark markType = Mark.Empty; /* Have to use old way of defining properties here unfortunately due to lack of support for,                                           default values in auto-implemented properties. */    public Mark MarkType {         get { return markType; }         set         {   //Only allow the type of the cell to change if it it's empty            if (MarkType == Mark.Empty)            {                markType = value;            }        }     }    #endregion    #region Constructors    public Cell()    {        MarkType = Mark.Empty;    }    #endregion}Grid.csenum OutcomeType{    None = -1, CrossWin, NoughtWin, Draw}class Grid{    const int MAX_CELLS = 3;    #region Properties & Fields    public Cell[,] cells { get; set; }    public OutcomeType Outcome { get; private set; }    #endregion    #region Constructors    public Grid()    {        Outcome = OutcomeType.None;        cells = new Cell[MAX_CELLS, MAX_CELLS];        for (int x = 0; x < MAX_CELLS; x++)        {            for (int y = 0; y < MAX_CELLS; y++)            {                cells[x, y] = new Cell();            }        }    }    #endregion    #region Instanced Methods    public bool CheckOutcome(Tuple<int, int> coords, Player player)    {        //Check for draws first        if (cells.GetEmptyCells().Length == 0)        {            Outcome = OutcomeType.Draw;               return true;        }        //Now check for player wins        var corners = new Tuple<int, int>[] { Tuple.Create(0,0), Tuple.Create(2,0), Tuple.Create(0,2), Tuple.Create(2,2) };        var sides = new Tuple<int, int>[] { Tuple.Create(1,0), Tuple.Create(0,1), Tuple.Create(1,2), Tuple.Create(2,1) };        var middle = new Tuple<int, int>(1,1);        var checkDiagonals = false;        //If the cell is at the corner or the middle we have to check for diagonal wins too        if (corners.Any(e => e.Equals(coords)) || middle.Equals(coords)) { checkDiagonals = true; }        if (player.PlayerWon(cells[coords.Item1, coords.Item2], this, checkDiagonals))        {            switch (player.marker)            {                case Mark.Cross:                    Outcome = OutcomeType.CrossWin;                    break;                case Mark.Nought:                    Outcome = OutcomeType.NoughtWin;                    break;            }            return true;        }        //If execution reaches this point then no one has won, return false        return false;    }   #endregion}Player.csclass Player{    #region Fields & Properties    private static int ctr;    private static Game form;    public Mark marker;    public readonly int ID;    public int Score { get; set; }    #endregion     #region Constructors    public Player()    {        ID = ctr++;        if (ID == 0) { // X is the starting player            marker = Mark.Cross; }         else {             marker = Mark.Nought; }    }    #endregion    #region Instanced Methods    public virtual Tuple<int, int> GetPlayerChoice(Grid grid, Player player) // Virtual because we want the option to override this for AIPlayer later.    {        var pMark = (player.ID == 0) ? X : O;        var choice = new Tuple<int,int>(-1, -1);        var waitBtns = new List<Task>() { new Task(() => new SpinWait().SpinOnce()) };        //Enable eligble buttons for player        for (int x = 0; x < grid.cells.GetLength(0); x++)        {            for (int y = 0; y < grid.cells.GetLength(1); y++)            {                if (grid.cells[x, y].MarkType == Mark.Empty)                {                    Game.vGrid[x, y].Invoke(new Action(() => Game.vGrid[x, y].Enabled = true));                                                    Game.vGrid[x, y].Click += new EventHandler(                        (a,b) =>                         {                            var pos = new Tuple<int,int>((a as VisualCell).CellPos.Item1, (a as VisualCell).CellPos.Item2);                            choice = Tuple.Create(pos.Item1, pos.Item2);                            Game.vGrid[pos.Item1, pos.Item2].Invoke(new Action(() => Game.vGrid[pos.Item1, pos.Item2].Text = pMark));                            Game.vGrid[pos.Item1, pos.Item2].Invoke(new Action(() => Game.vGrid[pos.Item1, pos.Item2].Enabled = false));                            waitBtns.Add(new Task(() => new SpinWait().SpinOnce()));                            waitBtns[waitBtns.FindIndex(t => t.Status == TaskStatus.Created)].Start();                        });                }            }                        }        Task.WaitAny(waitBtns.ToArray());        return choice;    }    public bool PlayerWon(Cell cell, Grid grid, bool checkDiagonals)     {        if (cell.HorizontalRelatives(grid) == 2 || cell.VerticalRelatives(grid) == 2)        {            return true;        }        if (checkDiagonals)        {            if (cell.DiagonalRelatives(grid) == 2) { return true; }             else if (cell.DiagonalRelatives2(grid) == 2) { return true; }        }        return false;    }    #endregion }Gameround.csclass GameRound{    public delegate void RoundEndHandler(OutcomeType gameOutcome);    #region Fields & Properties    private static int ctr;    private Grid grid;    public readonly int ID;    public static event RoundEndHandler Complete;    public Player CurrentPlayer { get; private set; }    #endregion    #region Constructors    public GameRound()    {        ID = ctr++;        grid = new Grid();    }    #endregion    #region Instanced Methods    public void Start()    {        bool doLoop = true;        while (doLoop)        {            for (int i = 0; i < Game.players.Length; i++)            {                CurrentPlayer = Game.players[i];                var coord = Game.players[i].GetPlayerChoice(grid, CurrentPlayer);                grid.cells[coord.Item1, coord.Item2].MarkType = CurrentPlayer.marker;                //check if game is over                if (grid.CheckOutcome(coord, CurrentPlayer))                  {                    doLoop = false;                    break;                 }            }        }        Complete(grid.Outcome);     }    #endregion}ExtensionMethods.csstatic class ExtensionMethods{    #region Cell Extension Methods    public static int DiagonalRelatives(this Cell cell, Grid grid)     {        int relatives = new int();        for (int x = 0; x < 3; x++)        {            if (grid.cells[x, x].MarkType.Equals(cell.MarkType)) { relatives++; }        }        return relatives - 1;    }    public static int DiagonalRelatives2(this Cell cell, Grid grid)    {        int relatives = new int();        for (int x = 0; x < 3; x++)        {            if (grid.cells[x, 2 - x].MarkType.Equals(cell.MarkType)) { relatives++; }        }        return relatives - 1;    }    public static int HorizontalRelatives(this Cell cell, Grid grid)    {        int relatives = new int();        int rowNum = grid.cells.IndexOf(cell).Item2;        for (int x = 0; x < 3; x++)        {            //Find row of cell            if (grid.cells[x, rowNum].MarkType.Equals(cell.MarkType)) { relatives++; }        }        return relatives - 1;    }    public static int VerticalRelatives(this Cell cell, Grid grid)    {        int relatives = new int();        int colNum = grid.cells.IndexOf(cell).Item1;        for (int y = 0; y < 3; y++)        {            //Find row of cell            if (grid.cells[colNum, y].MarkType.Equals(cell.MarkType)) { relatives++; }        }        return relatives - 1;    }    #endregion    #region Cell[] Extension Methods    public static Cell[] GetEmptyCells(this Cell[,] cells)    {        List<Cell> emptyCells = new List<Cell>();        foreach (Cell cell in cells)        {            if (cell.MarkType == Mark.Empty)            {                emptyCells.Add(cell);            }        }        return emptyCells.ToArray();    }    public static Tuple<int, int> IndexOf(this Cell[,] cells, Cell cell)     {        for (int x = 0; x < cells.GetLength(0); x++)        {            for (int y = 0; y < cells.GetLength(1); y++)            {                if (cells[x, y].Equals(cell))                {                    return new Tuple<int,int>(x, y);                }            }        }        //If code reaches this point, then it didn't find anything, return -1        return new Tuple<int,int>(-1, -1);    }    #endregion}
Clean and well-designed Tic-Tac-Toe game
c#;object oriented;multithreading;game
One more set of comments.You don't do a good job of invoking Dispose on objects whose classes implement IDisposable.This includes your many Task instances, and your many VisualCell instances (there may be others I didn't notice).I'd like to see more calls to Dispose, and/or using statements.You can dispute this advice:Tasks: Do I need to dispose of Tasks?VisualCell instances: they have the same lifetime as your main form, and the application will end (with all resources reclaimed by the O/S) when your main form closes.I mention it because it (disposing disposables) is IMO good practice.The code in GetPlayerChoice is IMO a horror show: masses of Invoke statements and Task instances with SpinWait instances.You're installing vGrid[x, y].Click event handlers each time you call GetPlayerChoice (and I don't understand what happens to old, previous-installed Click event handlers).The normal way to do this in a Windows Forms application would be:Don't use a TaskInstall a Click event handler once, in the Form (Game) classThe event handler could call a method of GameRound, e.g. a method named void OnPlayerChoice(VisualCell chosenCell)Game would need a reference to the GameRound instance: therefore the GameRound instance would be member data of Game, instead of just a local variable in the NewGame method, so that Game could call the GameRound instance's OnPlayerChoice methodOnPlayerChoice could return OutcomeType? in which case it wouldn't need a delegate to do a callback to the ShowCompletionDialogGameRound would need to remember which is the CurrentPlayer (as a property not just as a local variable); alternatively, Game could remember that, and pass it as a parameter. I still think that maybe GameRound should be merged into the Game class.Because there are no Task instances, you don't need to use Invoke. The game is event-driven from the GUI, i.e. driven by the Click event handlers.If one of the players is an AI then the Game, instead of waiting for the Click event, can call the AI player's choose method immediately after processing the human player's GUI click.Alternatively it's theoretically possible, e.g. by defining abstract interfaces between 'View' (Form) and 'Controller' (GameRound) and 'Data Model' (Grid and Cells), to decouple the Game or GameRound from the Form more completely: so that the GameRound doesn't know whether it's attached to a Form, or to a Console, or is simply writing results to a File.If you eventually did that, that might be another way to implement an AI: if the GameRound doesn't know where the player choices were coming from it might be easy to make some of the choices come from an AI instead of the GUI.isn't the point of the managed garbage collector to dispose of objects that aren't used anymore for me?Yes. The garbage collector might take a while to run, though. For example if you open a file and then let go of the open file without disposing it, the O/S file handle will remain open until the garbage collector reclaims the object; during that time, you can't reopen/reuse the file.Similarly some resources are in limited supply: you might only be allowed a few SQL connections, or SQL transaction, or GDI objects (brushes and fonts). A few might be hundreds, but if the garbage collector runs every 10 seconds or so that gives you a rate-limited throughput of 10 per second: so your code works fine until you put a real-world heavy load on it.The garbage collector is there to reclaim memory. It knows what memory you have used. If the memory contains undisposed resources those resources will be properly disposed; but I see that as a second line of defence (in case you didn't dispose them explicitly).Also about the old previous-installed event handlers for Click I also thought that would screw things up but having ran a dozen iterations of the game and seperate rounds without a problem, it doesn't seem to do so?If I put a breakpoint in the event handler I can see that it's called multiple times: once the first time, twice the second time, etc. If it's called more than once (e.g. twice) then it sets the vButton text twice, disables it twice, adds two tasks to the waitBtns list. The event handlers also occupy memory and can't be garbage-collected: so if you ran it for long enough (much longer than you've run it) it would run out of memory.Would you elaborate on what you mean by implementing abstract interfaces?The simplest version I can think of wouldn't have interfaces.Have a GameController with the following public API:// Let players playvoid PlayerChoose(Marker playerId, Tuple<int,int> position);// Let players inspect the current board// (optional, otherwise players can remember the state of play// e.g. the Game Form has this state already duplicated in its buttons)Cell[,] Cells { get; }// Tell players when the board has changeddelegate void PlayedEventHandler(Marker playerId, Tuple<int,int> position);event PlayedEventHandler Played;// Tell players when the game is finisheddelegate void FinishedEventHandler(OutcomeType outcome);event FinishedEventHandler Finished;// Let players start a new gamevoid Reset();An all-human Form-based application can:Have a Game Form like yoursGame contains (references) a GameControllerGame attaches to the Played event handler and updates its UI buttons accordinglyGame attaches to its buttons' Clicked event handler and invokes the PlayerChoose method accordinglyA human-versus-AI application can do the same as the above, except:The Game Form is only used by one playerThere's another, AI class, which:Contains a reference to the GameControllerAttaches to the Played event handlerChooses its next move and invokes the PlayerChoose method, when the Played event handler tells it that the human has played.See also:ModelviewpresenterOther tic-tac-toe implementations in c# on this site (I don't know which of them are good)