6
\$\begingroup\$

I took the source code from this question, (thank you gaessaki for the motivation!) and did a lot of refactoring.

I use 4 projects:

  • Common: contains mainly interfaces and enumerations referenced by other projects.
  • Model: Knows nothing of the others, just maintains the game logic and state.
  • Presenter: References the Model directly and accesses the View via the IView interface. Acts as mediator between them.
  • View: References the Presenter. Is pretty dumb, can only query the board situation and the game status and display them. For every action (playing a move, restarting, etc.) it simply informs the presenter.

The View is actually the executable project who references all others and in its Program.cs instantiates a Model, a View, a Presenter and connects them. But other than that, the visual components only touch the presenter. I did not want to create another project just to this instantiation.

The intention is to be able to interchange the view (e.g. use console or WPF), keeping the model and the presenter.

I appreciate your comments. I have experience with C# and programming in general, but not with MVP. I think, I tend to overcomplicate the design.

Help classes

ExceptionBuilder

using System;

namespace Mfanou.Common {
    public static class ExceptionBuilder {
        public static void CheckArgumentRangeInclusive(string varName, int value, int lowerRange, int upperRange) {
            if (value < lowerRange || value > upperRange)
                throw new ArgumentOutOfRangeException(varName);
        }
    }
}

Common

GameAction enumeration

namespace Mfanou.TicTacToe.Common {
    public enum GameAction {
        Restart,
        Exit
    }
}

Move enumeration

namespace Mfanou.TicTacToe.Common {
    public enum Move {
        ShowPreview,
        HidePreview,
        Play
    }
}

IPlayer

namespace Mfanou.TicTacToe.Common {
    public interface IPlayer {
        int Id { get; }
    }
}

ISquareContent

namespace Mfanou.TicTacToe.Common {
    public interface ISquareContent {
        bool IsEmpty { get; }

        /// <summary>
        /// Player whose piece is on the square.
        /// Valid only when IsEmpty is false.
        /// </summary>
        IPlayer Player { get; }

        /// <summary>
        /// True if the piece is a move preview.
        /// Valid only when IsEmpty is false.
        /// </summary>
        bool IsPiecePreview { get; }

        /// <summary>
        /// True if the piece is part of a game-winning piece sequence.
        /// Valid only when IsEmpty is false.
        /// </summary>
        bool IsWinning { get; }
    }
}

SquarePosition

using Mfanou.Common;

    namespace Mfanou.TicTacToe.Common {
        public class SquarePosition {
            public static readonly int ROWCOL_MIN = 1;
            public static readonly int ROWCOL_MAX = 3;

            public SquarePosition(int row, int col) {
                CheckRowColRange(nameof(row), row);
                CheckRowColRange(nameof(col), col);

                Row = row;
                Column = col;
            }

            public int Row { get; }
            public int Column { get; }

            public override bool Equals(object obj) {
                if (obj == null || GetType() != obj.GetType()) 
                    return false;

                return Equals((SquarePosition)obj);
            }
            public bool Equals(SquarePosition sp) => Row == sp.Row && Column == sp.Column;
            public override int GetHashCode() => 1024 * Row.GetHashCode() + Column.GetHashCode();

            private void CheckRowColRange(string varName, int value) {
                ExceptionBuilder.CheckArgumentRangeInclusive(varName, value, ROWCOL_MIN, ROWCOL_MAX);
            }
        }
    }

IGameStatus

namespace Mfanou.TicTacToe.Common {
    public interface IGameStatus {
        bool IsOver { get; }

        /// <summary>Valid only when IsEmpty is true.</summary>
        bool IsTie { get; }

        /// <summary>Valid only when IsEmpty is true and IsTie is false.</summary>
        IPlayer WinningPlayer { get; }
    }
}

IView

namespace Mfanou.TicTacToe.Common {
    public interface IView {
        void RefreshBoard();

        bool ConfirmAction(GameAction action);

        void Exit();
    }
}

Model

Game (main class, it being the actual model)

using Mfanou.TicTacToe.Common;
using System;

namespace Mfanou.TicTacToe.Model {
    public class Game {
        public Game() {
            MoveFactory = new MoveFactory(this);
            GameActionFactory = new GameActionFactory(this);

            Board = new Board();
            Turn = new Turn<IPlayer>(Player.GetAll());

            new RestartAction(this).Execute();
        }

        public event Action OnExit;

        public IGameStatus Status => InternalStatus;

        public MoveFactory MoveFactory { get; private set; }
        public GameActionFactory GameActionFactory { get; private set; }

        public ISquareContent GetSquareContent(SquarePosition position) => Board.GetSquare(position).Content;


        internal Board Board { get; }

        internal Turn<IPlayer> Turn { get; }

        internal GameStatus InternalStatus { get; set; }


        internal void Exit() {
            OnExit?.Invoke();
        }

        internal void UpdateStatus() {
            InternalStatus = StatusJudge.GetStatus(Board);
        }
    }
}

Board

using Mfanou.TicTacToe.Common;
using System.Collections.Generic;
using System.Linq;

namespace Mfanou.TicTacToe.Model {
    internal class Board {
        public Board() {
            _squares = new List<Square>();

            Reset();
        }

        public IEnumerable<Square> Squares => _squares;
        public IEnumerable<IEnumerable<Square>> RowsColumnsAndDiagonals => Rows.Concat(Columns).Concat(Diagonals);

        public Square GetSquare(SquarePosition position) => 
            _squares.Where(sq => sq.Position.Equals(position)).Single();

        public void Reset() {
            _squares.Clear();
            for (int r = SquarePosition.ROWCOL_MIN; r <= SquarePosition.ROWCOL_MAX; r++)
                for (int c = SquarePosition.ROWCOL_MIN; c <= SquarePosition.ROWCOL_MAX; c++)
                    _squares.Add(new Square(new SquarePosition(r, c)));
        }

        private List<Square> _squares;

        private IEnumerable<IEnumerable<Square>> Rows => Squares.GroupBy(sq => sq.Position.Row);
        private IEnumerable<IEnumerable<Square>> Columns => Squares.GroupBy(sq => sq.Position.Column);
        private IEnumerable<IEnumerable<Square>> Diagonals {
            get {
                // Top left - bottom right diagonal: row equals column.
                yield return Squares.Where(sq => sq.Position.Row == sq.Position.Column);

                // Bottom left - top right diagonal: sum of row and column is constant.
                yield return Squares.Where(sq => sq.Position.Row + sq.Position.Column == 
                    SquarePosition.ROWCOL_MAX + SquarePosition.ROWCOL_MIN);
            }
        }
    }
}

GameAction subdir

To the GameAction enumeration in common corresponds a simple hierarchy of action classes deriving from the GameAction abstract class.

The Presenter uses the GameFactory provided by Game to translate the GameAction enum to a GameAction descendant class and then ask for a confirmation and/or execute it.

GameAction

using System.Linq;

namespace Mfanou.TicTacToe.Model {
    public abstract class GameAction {
        public GameAction(Game game) {
            Game = game;
        }

        public bool NeedsConfirmation() {
            if (Game.InternalStatus.IsOver)
                return false;

            bool boardHasMoves = Game.Board.Squares.Any((sq) => sq.Content.HasMove);
            return boardHasMoves;
        }

        public abstract void Execute();

        protected Game Game { get; }
    }
}

ExitAction

namespace Mfanou.TicTacToe.Model {
    internal class ExitAction : GameAction {
        internal ExitAction(Game game) : base(game) {}

        public override void Execute() {
            Game.Exit();
        }
    }
}

RestartAction

namespace Mfanou.TicTacToe.Model {
    internal class RestartAction : GameAction {
        internal RestartAction(Game game) : base(game) {}

        public override void Execute() {
            Game.Board.Reset();
            Game.Turn.Reset();
            Game.UpdateStatus();
        }
    }
}

GameActionFactory

using System;

namespace Mfanou.TicTacToe.Model {
    public class GameActionFactory {
        public GameActionFactory(Game game) {
            _game = game;
        }

        public GameAction CreateGameAction(Common.GameAction action) {
            GameAction gameAction;
            switch (action) {
                case Common.GameAction.Restart:
                    gameAction = new RestartAction(_game);
                    break;

                case Common.GameAction.Exit:
                    gameAction = new ExitAction(_game);
                    break;

                default:
                    throw new NotImplementedException();
            }

            return gameAction;
        }

        private Game _game;
    }
}

GameStatus subdir

GameStatus

using Mfanou.TicTacToe.Common;
using System;
using System.Collections.Generic;

namespace Mfanou.TicTacToe.Model {
    internal class GameStatus : IGameStatus {
        public static GameStatus Running() {
            return new GameStatus() { _isOver = false };
        }
        public static GameStatus Tie() {
            return new GameStatus() { _isOver = true, _isTie = true };
        }
        public static GameStatus Winner(IPlayer winner, IEnumerable<SquarePosition> winningSquares) {
            return new GameStatus() { _isOver = true, _isTie = false, _winner = winner, _winningSquares = winningSquares };
        }

        public bool IsOver => _isOver;
        /// <exception cref="InvalidOperationException">When game is not over.</exception>
        public bool IsTie {
            get {
                if (!IsOver)
                    throw new InvalidOperationException();
                return _isTie;
            }
        }
        /// <exception cref="InvalidOperationException">When game is not over, or is a tie.</exception>
        public IPlayer WinningPlayer {
            get {
                if (!IsOver || IsTie)
                    throw new InvalidOperationException();
                return _winner;
            }
        }
        /// <exception cref="InvalidOperationException">When game is not over, or is a tie.</exception>
        public IEnumerable<SquarePosition> WinningSquares {
            get {
                if (!IsOver || IsTie)
                    throw new InvalidOperationException();
                return _winningSquares;
            }
        }

        private GameStatus() {}

        private bool _isOver;
        private bool _isTie;
        private IPlayer _winner;
        private IEnumerable<SquarePosition> _winningSquares;
    }
}

StatusJudge

using Mfanou.TicTacToe.Common;
using System.Collections.Generic;
using System.Linq;

namespace Mfanou.TicTacToe.Model {
    /// <summary>Contains logic for getting the game status.</summary>
    internal static class StatusJudge {
        public static GameStatus GetStatus(Board board) {
            // For each row, column, diagonal...
            foreach (IEnumerable<Square> squares in board.RowsColumnsAndDiagonals) {
                IEnumerable<SquarePosition> positions = squares.Select((sq) => sq.Position);
                IEnumerable<SquareContent> contents = squares.Select((sq) => sq.Content);

                // ...if all its squares are covered by the same player, it's a win.
                SquareContent singleContent =
                    contents.Distinct().Count() == 1 ? contents.First() : null;
                if (singleContent != null && singleContent.HasMove)
                    return GameStatus.Winner(singleContent.Player, positions);
            }

            bool isBoardFull = board.Squares.All((sq) => sq.Content.HasMove);
            return isBoardFull ? GameStatus.Tie() : GameStatus.Running();
        }
    }
}

Move subdir

To the Move enumeration in Common corresponds here a class hierarchy deriving from the abstract Move class. The logic whether the move is allowed and its effects is thus extracted from the Game class.

The presenter uses the MoveFactory provided from the Game to translate the Move enumeration to a Move class descendant which then can query for allowing and executing a move requested by the form.

Move

using Mfanou.TicTacToe.Common;
using System;

namespace Mfanou.TicTacToe.Model {
    public abstract class Move {
        public Move(Game game, SquarePosition position) {
            Game = game;
            Position = position;
        }

        public SquarePosition Position { get; }

        public abstract bool CanExecute();
        public void Execute() {
            if (!CanExecute())
                throw new InvalidOperationException();

            DoExecute();
        }

        protected Game Game;

        protected abstract void DoExecute();
    }
}

ShowPreviewMove

using Mfanou.TicTacToe.Common;
using System.Linq;

namespace Mfanou.TicTacToe.Model {
    internal class ShowPreviewMove : Move {
        public ShowPreviewMove(Game game, SquarePosition position) : base(game, position) {}

        public override bool CanExecute() {
            // No other preview should exist on the board.
            if (Game.Board.Squares.Any(sq => !sq.Content.IsEmpty && sq.Content.IsPiecePreview))
                return false;

            // Square should be empty.
            return Game.Board.GetSquare(Position).Content.IsEmpty;
        }

        protected override void DoExecute() {
            Game.Board.GetSquare(Position).Content = SquareContent.WithPiecePreview(Game.Turn.Current);
        }
    }
}

HidePreviewMove

using Mfanou.TicTacToe.Common;

namespace Mfanou.TicTacToe.Model {
    internal class HidePreviewMove : Move {
        public HidePreviewMove(Game game, SquarePosition position) : base(game, position) {}

        public override bool CanExecute() {
            var targetContent = Game.Board.GetSquare(Position).Content;
            return !targetContent.IsEmpty && targetContent.IsPiecePreview;
        }

        protected override void DoExecute() {
            Game.Board.GetSquare(Position).Content = SquareContent.Empty();
        }
    }
}

PlayMove

using Mfanou.TicTacToe.Common;
using System.Linq;

namespace Mfanou.TicTacToe.Model {
    internal class PlayMove : Move {
        public PlayMove(Game game, SquarePosition position) : base(game, position) {}

        public override bool CanExecute() {
            // If there is a preview, it can only be played in the previewed square.
            var previewSquare = Game.Board.Squares.
                Where(sq => !sq.Content.IsEmpty && sq.Content.IsPiecePreview).SingleOrDefault();
            if (previewSquare != null) 
                return previewSquare.Position.Equals(Position);

            // No preview: It can only be played in an empty square.
            return Game.Board.GetSquare(Position).Content.IsEmpty;
        }

        protected override void DoExecute() {
            Game.Board.GetSquare(Position).Content = SquareContent.WithPiece(Game.Turn.Current);

            Game.UpdateStatus();
            if (Game.InternalStatus.IsOver) {
                // If this move just won the game, highlight the winning squares.
                if (!Game.InternalStatus.IsTie)
                    Game.InternalStatus.WinningSquares.ToList().
                        ForEach(sp => Game.Board.GetSquare(sp).Content = SquareContent.WithPieceWinning(Game.Turn.Current));
            } else
                Game.Turn.MoveToNext();
        }
    }
}

MoveFactory

using Mfanou.TicTacToe.Common;
using System;

namespace Mfanou.TicTacToe.Model {
    public class MoveFactory {
        public MoveFactory(Game game) {
            _game = game;
        }

        public Move CreateMove(Common.Move action, SquarePosition position) {
            Move move;
            switch (action) {
                case Common.Move.ShowPreview:
                    move = new ShowPreviewMove(_game, position);
                    break;
                case Common.Move.HidePreview:
                    move = new HidePreviewMove(_game, position);
                    break;

                case Common.Move.Play:
                    move = new PlayMove(_game, position);
                    break;

                default:
                    throw new NotImplementedException();
            }

            return move;
        }

        private Game _game;
    }
}

Player subdir

Player

using Mfanou.TicTacToe.Common;
using System.Collections.Generic;

namespace Mfanou.TicTacToe.Model {
    internal class Player : IPlayer {
        public static IEnumerable<IPlayer> GetAll() {
            for (int i=1; i<=NUM_PLAYERS; i++)
                yield return new Player(i);
        }

        public int Id { get; }

        public override bool Equals(object obj) {
            if (obj == null || GetType() != obj.GetType()) 
                return false;

            return Equals((Player)obj);
        }
        public bool Equals(Player p) {
            return (Id == p.Id);
        }
        public override int GetHashCode() {
            return Id.GetHashCode();
        }

        private static readonly int NUM_PLAYERS = 2;

        private Player(int id) {
            Id = id;
        }
    }
}

Turn

using Mfanou.TicTacToe.Common;
using System.Collections.Generic;
using System.Linq;

namespace Mfanou.TicTacToe.Model {
    internal class Turn<T> {
        public Turn(IEnumerable<T> players) {
            _players = players.ToArray();
            Reset();
        }

        public IEnumerable<T> Players => _players;
        public T Current => _players[_indexCurrent];

        public void Reset() {
            _indexCurrent = 0;
        }
        public void MoveToNext() {
            _indexCurrent++;
            if (_indexCurrent >= _players.Length)
                _indexCurrent = 0;
        }

        private T[] _players;
        private int _indexCurrent;
    }
}

Square subdir

Square

using Mfanou.TicTacToe.Common;

namespace Mfanou.TicTacToe.Model {
    internal class Square {
        public Square(SquarePosition position) {
            Position = position;
            Content = SquareContent.Empty();
        }

        public SquarePosition Position { get; }
        public SquareContent Content { get; set; }
    }
}

SquareContent

using Mfanou.TicTacToe.Common;
using System;

namespace Mfanou.TicTacToe.Model {
    internal class SquareContent : ISquareContent {
        public static SquareContent Empty() {
            return new SquareContent() { IsEmpty = true };
        }
        public static SquareContent WithPiece(IPlayer player) {
            return new SquareContent() { IsEmpty = false, Player = player };
        }
        public static SquareContent WithPiecePreview(IPlayer player) {
            return new SquareContent() { IsEmpty = false, Player = player, IsPiecePreview = true };
        }
        public static SquareContent WithPieceWinning(IPlayer player) {
            return new SquareContent() { IsEmpty = false, Player = player, IsWinning = true };
        }

        public bool IsEmpty { get; private set; }
        public bool HasMove => !IsEmpty && !IsPiecePreview;
        /// <exception cref="InvalidOperationException">When the square is empty.</exception>
        public IPlayer Player {
            get {
                if (IsEmpty)
                    throw new InvalidOperationException();
                return _player;
            }
            private set { _player = value; }
        }
        public bool IsPiecePreview {
            get {
                if (IsEmpty)
                    throw new InvalidOperationException();
                return _isPreview;
            }
            private set { _isPreview = value; }
        }
        public bool IsWinning {
            get {
                if (IsEmpty)
                    throw new InvalidOperationException();
                return _isWinning;
            }
            private set { _isWinning = value; }
        }

        public override bool Equals(object obj) {
            if (obj == null || GetType() != obj.GetType()) 
                return false;

            return Equals((SquareContent)obj);
        }
        public bool Equals(SquareContent sc) {
            if (IsEmpty && sc.IsEmpty)
                return true;

            if (!IsEmpty && !sc.IsEmpty && Player == sc.Player && IsPiecePreview == sc.IsPiecePreview)
                return true;

            // Exactly one of {this,sc} IsEmpty.
            return false;
        }
        public override int GetHashCode() {
            return IsEmpty ? IsEmpty.GetHashCode() : IsEmpty.GetHashCode() * 1024 + Player.GetHashCode();
        }

        public SquareContent Clone() {
            var sc = new SquareContent();
            sc.IsEmpty = IsEmpty;
            if (!IsEmpty) {
                sc.Player = Player;
                sc.IsPiecePreview = IsPiecePreview;
                sc.IsWinning = IsWinning;
            }
            return sc;
        }

        private SquareContent() {}

        private IPlayer _player;
        private bool _isPreview;
        private bool _isWinning;
    }
}

Presenter

GamePresenter

using Mfanou.TicTacToe.Common;
using Mfanou.TicTacToe.Model;

namespace Mfanou.TicTacToe.Presenter {
    public class GamePresenter {
        public GamePresenter(Game model, IView view) {
            Model = model;
            Model.OnExit += ExitGame;

            View = view;
        }

        public IGameStatus GameStatus => Model.Status;
        public ISquareContent GetSquareContent(SquarePosition position) => Model.GetSquareContent(position);

        public void RequestAction(Common.GameAction action) {
            Model.GameAction gameAction = Model.GameActionFactory.CreateGameAction(action);

            if (gameAction.NeedsConfirmation() && !View.ConfirmAction(action))
                return;

            gameAction.Execute();
            View.RefreshBoard();
        }
        public void RequestMove(Common.Move action, SquarePosition position) {
            Model.Move move = Model.MoveFactory.CreateMove(action, position);

            if (!move.CanExecute())
                return;

            move.Execute();
            View.RefreshBoard();
        }

        private Game Model { get; set; }
        private IView View { get; set; }

        private void ExitGame() {
            View.Exit();
        }
    }
}

View

Program.cs

using Mfanou.TicTacToe.Model;
using Mfanou.TicTacToe.Presenter;
using System;
using System.Windows.Forms;

namespace Mfanou.TicTacToe.UI.WinForms {
    internal static class Program {
        [STAThread]
        static void Main() {
            Application.EnableVisualStyles();
            Application.SetCompatibleTextRenderingDefault(false);
            Application.Run(CreateMainForm());
        }

        static Form CreateMainForm() {
            var model = new Game();
            var view = new TicTacToeForm();
            var presenter = new GamePresenter(model, view);
            view.Presenter = presenter;

            return view;
        }
    }
}

TicTacToeForm

using Mfanou.TicTacToe.Common;
using Mfanou.TicTacToe.Presenter;
using Mfanou.UI.Winforms;
using System;
using System.Drawing;
using System.Windows.Forms;

namespace Mfanou.TicTacToe.UI.WinForms {
    public partial class TicTacToeForm : MyForm, IView {
        public TicTacToeForm() {
            InitializeComponent();

            Size = VisualFormatter.FormDefaultSize;
            MinimumSize = VisualFormatter.FormMinimumSize;
            Text = VisualFormatter.GAME_TITLE;

            CreateMainPanel();
            CreateMenu();

            FormClosing += Form_Closing;
        }

        public GamePresenter Presenter {
            get {
                return _presenter;
            }
            set {
                _presenter = value;
                RefreshBoard();
            }
        }

        public bool ConfirmAction(GameAction action) => VisualFormatter.ConfirmAction(action);

        public void RefreshBoard() {
            var status = Presenter.GameStatus;

            BoardPanel.Enabled = !status.IsOver;
            foreach (Control ctrl in BoardPanel.Controls)
                VisualFormatter.FormatSquare(ctrl, Presenter.GetSquareContent(GetSquarePosition(ctrl)));

            ResultLabel.Text = VisualFormatter.GameResult(status);
        }

        public void Exit() {
            _formOrderedToClose = true;
            Close();
        }


        private GamePresenter _presenter;

        private bool _formOrderedToClose = false;
        private TableLayoutPanel BoardPanel;
        private Label ResultLabel;

        private void CreateMenu() {
            var RestartSubmenuItem = new ToolStripMenuItem() {
                Text = "&Restart",
                ShortcutKeys = Keys.Control | Keys.N,
            };
            var ExitSubmenuItem = new ToolStripMenuItem() {
                ShortcutKeys = Keys.Control | Keys.X,
                Text = "E&xit",
            };
            var GameMenuItem = new ToolStripMenuItem() { Text = "&Game" };
            GameMenuItem.DropDownItems.AddRange(new ToolStripItem[] { RestartSubmenuItem, ExitSubmenuItem });

            var LicenseSubmenuItem = new ToolStripMenuItem() { Text = "&License" };
            var HelpMenuItem = new ToolStripMenuItem() { Text = "&Help" };
            HelpMenuItem.DropDownItems.AddRange(new ToolStripItem[] { LicenseSubmenuItem });

            MainMenuStrip = new MenuStrip();
            Controls.Add(MainMenuStrip);
            MainMenuStrip.Items.AddRange(new ToolStripItem[] { GameMenuItem, HelpMenuItem });

            RestartSubmenuItem.Click += RestartToolStripMenuItem_Click;
            ExitSubmenuItem.Click += ExitToolStripMenuItem_Click;
            LicenseSubmenuItem.Click += LicenseToolStripMenuItem_Click;
        }
        private void CreateMainPanel() {
            var MainPanel = new TableLayoutPanel() {
                Dock = DockStyle.Fill,
                Margin = new Padding(0)
            };
            Controls.Add(MainPanel);
            MainPanel.RowStyles.Add(new RowStyle(SizeType.Percent, 100));
            MainPanel.RowStyles.Add(new RowStyle(SizeType.Absolute, 20));

            BoardPanel = CreateBoardPanel();
            MainPanel.Controls.Add(BoardPanel, column: 0, row: 0);

            ResultLabel = CreateResultLabel();
            MainPanel.Controls.Add(ResultLabel, column: 0, row: 1);
        }
        private TableLayoutPanel CreateBoardPanel() {
            var panel = new TableLayoutPanel() {
                Dock = DockStyle.Fill,
                Margin = new Padding(0),
            };

            int size = SquarePosition.ROWCOL_MAX - SquarePosition.ROWCOL_MIN + 1;
            Construct2DGridInPanel(panel, size);

            return panel;
        }
        private void Construct2DGridInPanel(TableLayoutPanel panel, int size) {
            panel.ColumnCount = size;
            panel.RowCount = size;

            for (int i = 0; i < size; i++) {
                panel.ColumnStyles.Add(new ColumnStyle(SizeType.Percent, 100));
                panel.RowStyles.Add(new RowStyle(SizeType.Percent, 100));
            }

            for (int row = 0; row < size; row++)
                for (int col = 0; col < size; col++)
                    panel.Controls.Add(CreateSquare(), row, col);
        }
        private Control CreateSquare() {
            var square = new Button() {
                FlatStyle = FlatStyle.Popup,
                Dock = DockStyle.Fill,
                Font = VisualFormatter.SquareFont,
            };

            square.MouseEnter += Square_MouseEnter;
            square.MouseLeave += Square_MouseLeave;
            square.MouseClick += Square_MouseClick;

            return square;
        }
        private Label CreateResultLabel() {
            return new Label() {
                AutoSize = true,
                Dock = DockStyle.Left,
                Font = VisualFormatter.ResultLabelFont,
                ForeColor = Color.Red,
            };
        }

        private SquarePosition GetSquarePosition(Control ctrl) {
            return new SquarePosition(
                SquarePosition.ROWCOL_MIN + BoardPanel.GetRow(ctrl), 
                SquarePosition.ROWCOL_MIN + BoardPanel.GetColumn(ctrl)
            );
        }

        private void Form_Closing(object sender, FormClosingEventArgs e) {
            // Form allowed to close only if ordered by presenter.
            if (!_formOrderedToClose) {
                Presenter.RequestAction(GameAction.Exit);
                e.Cancel = true;
            }
        }

        private void Square_MouseEnter(object sender, EventArgs e) {
            Presenter.RequestMove(Common.Move.ShowPreview, GetSquarePosition(sender as Control));
        }
        private void Square_MouseLeave(object sender, EventArgs e) {
            Presenter.RequestMove(Common.Move.HidePreview, GetSquarePosition(sender as Control));
        }
        private void Square_MouseClick(object sender, MouseEventArgs e) {
            Presenter.RequestMove(Common.Move.Play, GetSquarePosition(sender as Control));
        }

        private void RestartToolStripMenuItem_Click(object sender, EventArgs e) {
            Presenter.RequestAction(GameAction.Restart);
        }
        private void ExitToolStripMenuItem_Click(object sender, EventArgs e) {
            Presenter.RequestAction(GameAction.Exit);
        }
        private void LicenseToolStripMenuItem_Click(object sender, EventArgs e) {
            new LicenseForm().ShowDialog();
        }
    }
}

LicenseForm

using System;
using System.IO;

    namespace Mfanou.TicTacToe.UI.WinForms {
        internal partial class LicenseForm : MyForm {
            public LicenseForm() {
                InitializeComponent();

                Text = "License";

                textBoxLicense.Text = File.ReadAllText(
                    Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Resources", "License.txt"));
            }
        }
    }

VisualFormatter

using Mfanou.TicTacToe.Common;
using System;
using System.Collections.Generic;
using System.Drawing;
using System.Linq;
using System.Windows.Forms;

namespace Mfanou.TicTacToe.UI.WinForms {
    internal class VisualFormatter {
        public static readonly string GAME_TITLE = "Tic Tac Toe";

        public static Font SquareFont => new Font("Arial", 48F, FontStyle.Regular, GraphicsUnit.Point, 0); 
        public static Font ResultLabelFont => new Font("Microsoft Sans Serif", 10F, FontStyle.Bold, GraphicsUnit.Point, 0);

        public static Size FormDefaultSize = new Size(500, 549);
        public static Size FormMinimumSize = new Size(331, 362);

        public static void FormatSquare(Control square, ISquareContent content) {
            Color STANDARD_FCOLOR = SystemColors.ControlText;
            Color STANDARD_BCOLOR = SystemColors.Window;

            if (content.IsEmpty) {
                square.Text = string.Empty;
                square.ForeColor = STANDARD_FCOLOR;
                square.BackColor = STANDARD_BCOLOR;
            } else {
                VisualPlayer player = ToVisualPlayer(content.Player);
                square.Text = player.BoardSquareMark.ToString();
                square.ForeColor = content.IsPiecePreview ? player.MovePreviewForeColor : player.MoveForeColor;
                square.BackColor = content.IsWinning ? player.WinBackColor : STANDARD_BCOLOR;
            }
        }

        public static bool ConfirmAction(GameAction action) {
            const string CONFIRMATION = "Confirmation";
            const string GAME_NOT_OVER = "Game is not over.\nAre you sure you want to {0}?";
            var GameActionDescr = new Dictionary<GameAction, string>() {
                { GameAction.Restart, "restart" },
                { GameAction.Exit, "exit" },
            };

            return MessageBox.Show(
                string.Format(GAME_NOT_OVER, GameActionDescr[action]), CONFIRMATION,
                MessageBoxButtons.YesNo, MessageBoxIcon.Question, MessageBoxDefaultButton.Button2
            ) == DialogResult.Yes;
        }

        public static string GameResult(IGameStatus status) {
            const string GAMERESULT_TIE = "It's a tie!";
            const string GAMERESULT_WINNER = "Player {0} wins!";

            if (!status.IsOver)
                return string.Empty;
            if (status.IsTie)
                return GAMERESULT_TIE;
            return string.Format(GAMERESULT_WINNER, ToVisualPlayer(status.WinningPlayer).Name);
        }


        private static VisualPlayer ToVisualPlayer(IPlayer player) {
            var VisualPlayers = new List<VisualPlayer>() {
                new VisualPlayer() {
                        Id = 1,
                        Name = "X", BoardSquareMark = 'X',
                        MovePreviewForeColor = Color.LightBlue,
                        MoveForeColor = Color.Blue,
                        WinBackColor = Color.LightBlue,
                },
                new VisualPlayer() {
                        Id = 2,
                        Name = "O", BoardSquareMark = 'O',
                        MovePreviewForeColor = Color.LightCoral,
                        MoveForeColor = Color.Crimson,
                        WinBackColor = Color.LightCoral,
                },
                new VisualPlayer() {
                        Id = 3,
                        Name = "+", BoardSquareMark = '+',
                        MovePreviewForeColor = Color.LightGreen,
                        MoveForeColor = Color.Green,
                        WinBackColor = Color.LightGreen,
                },
                new VisualPlayer() {
                        Id = 4,
                        Name = "$", BoardSquareMark = '$',
                        MovePreviewForeColor = Color.PaleTurquoise,
                        MoveForeColor = Color.DarkTurquoise,
                        WinBackColor = Color.PaleTurquoise,
                },
            };

            if (VisualPlayers.Count(vp => vp.Id == player.Id) == 0)
                throw new ArgumentOutOfRangeException();

            return VisualPlayers.Single(vp => vp.Id == player.Id);
        }
    }
}

VisualPlayer

using Mfanou.TicTacToe.Common;
using System.Drawing;

namespace Mfanou.TicTacToe.UI.WinForms {
    internal class VisualPlayer : IPlayer {
        public int Id { get; set; }

        public string Name;
        public char BoardSquareMark;

        public Color MovePreviewForeColor;
        public Color MoveForeColor;
        public Color WinBackColor;
    }
}

My general comments:

  • The messiest thing seems to be the TicTacToeForm. I experimented with code (instead of using the designer) for creating the menu and the grid of the board.
  • ShowLicense should probably be an action, but then it should be a third kind.
  • I try to keep code explaining itself. So I used comments only where I thought that it was not very clear from the code itself and/or the class/method/variable names of what happens.

I have in my todo list to extract all strings (including menu captions) to a resource and write unit tests (none yet. q-:)

(This is a long question. Thank you anyway for reaching that far!)

P.S.: After reading the code here myself:

  • Instead of exposing GameActionFactory and MoveFactory in Model, I could simply expose the functions CreateGameAction and CreateMove. 2 public classes less.
\$\endgroup\$
  • 2
    \$\begingroup\$ Haven't looked at the code [yet], but given all that separation of concerns and decoupling, here's a challenge: how much of it needs to change for the game to become Ultimate Tic-Tac-Toe? ;-) \$\endgroup\$ – Mathieu Guindon Mar 30 '17 at 22:37
  • \$\begingroup\$ It's certainly not a trivial change. I noted down the task. :) \$\endgroup\$ – Manolis Mar 31 '17 at 22:04
  • \$\begingroup\$ One more interesting variation: The bidding tic-tac-toe: math.stackexchange.com/questions/1093308/bidding-tic-tac-toe \$\endgroup\$ – Manolis Apr 27 '17 at 17:42
5
\$\begingroup\$

To begin with...

public static class ExceptionBuilder {
    public static void CheckArgumentRangeInclusive(string varName, int value, int lowerRange, int upperRange) {
        if (value < lowerRange || value > upperRange)
            throw new ArgumentOutOfRangeException(varName);
    }
}

This is not a builder. It's a validator so I suggest naming it like ArgumentValidator and the method ValidateArgumentRangeInclusive.


public enum Move {
    ShowPreview,
    HidePreview,
    Play
}

To me, ShowPreview and HidePreview are rather view options then something that has anything to do with Move.


public static readonly int ROWCOL_MIN = 1;
public static readonly int ROWCOL_MAX = 3;

We don't use UPPER_CASE for constants in C# and the name ROWCOL isn't clear. Is it row or column? You can put them inside a static class to give the a better meaning.


private void CheckRowColRange(string varName, int value) {
    ExceptionBuilder.CheckArgumentRangeInclusive(varName, value, ROWCOL_MIN, ROWCOL_MAX);
}

This method can be made static because it does not require any state information from the owning class.


public interface IGameStatus {
    bool IsOver { get; }

    /// <summary>Valid only when IsEmpty is true.</summary>
    bool IsTie { get; }

    /// <summary>Valid only when IsEmpty is true and IsTie is false.</summary>
    IPlayer WinningPlayer { get; }
}

There is no IsEmpty.


public Game() {
    MoveFactory = new MoveFactory(this);
    GameActionFactory = new GameActionFactory(this);

    Board = new Board();
    Turn = new Turn<IPlayer>(Player.GetAll());

    new RestartAction(this).Execute();
}

The constructor should not be doing any thing but initializing data. Something like new RestartAction(this).Execute(); is a very bad idea and I'd be really surprised when I created an instance of a Game and it already did something even though it's not fully created yet. What's even worse, the RestartAction dependency is not passed as a such via the costructor so there is no way to override it for testing.


public IGameStatus Status => InternalStatus;
internal GameStatus InternalStatus { get; set; }

This doesn't seem right. Why would you make an internal status settable and the public one not? This looks like hacking something.

\$\endgroup\$
  • \$\begingroup\$ I guess renaming the Move hierarchy to MoveAction, ShowMovePreview, HideMovePreview, PlayMove solves the relevant issue. (You are absolutely right! A fish is an animal. A move preview is not a move.) \$\endgroup\$ – Manolis Mar 31 '17 at 14:02
  • \$\begingroup\$ Does it make sense the way I am using the MoveFactory? The View needs somehow to distinguish between (show/hide preview, play move), so it uses the Move enumeration. The presenter translates an enumeration member to a Move class descendant through the MoveFactory provided by the Model, and knows how to handle it (query if executable and execute). There is a name clash between the enumeration and the abstract base class Move which I don't like. Any hint for a better/simpler design? (Analogous stuff applies to GameAction for Exit and Restart). \$\endgroup\$ – Manolis Mar 31 '17 at 16:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.