You're using extension methods for cells; why do use for
loops for rows? It strikes me as weird. Also, there's plenty of repetition in your analyzer; you can do better by abstracting some things away (more on that below).
To answer your main question, instead of returning bool
for whether somebody won, you can return Nullable<Player>
where Player
is an enum with X
and O
values. This would not in any way violate single responsibility principle because your class' responsibility is to analyze the game, and determine the winner. There is no point in finding out if somebody has won, if you don't know who that somebody is.
That being said, I think you could still do better with the implementation. Your GameEndAnalyzer
analyzes each outcome manually, whereas you could've made it more generic by introducing a concept of “lines” and a few extension methods (somewhat less than you seem to have).
I re-wrote my earlier toy Tic Tac Toe implementation specifically to address your question, with the following separation:
Player
is a simple enum:
enum Player {
X,
O
}
IBoard
represents a board with an indexer by row
and column
and Size
.
interface IBoard
{
int Size { get; }
Player? this [int row, int column] { get; set; }
}
(really, you don't need anything else in IBoard
)
The implementation is trivial:
class Board : IBoard
{
Player? [,] _cells;
public int Size {
get { return _cells.GetLength (0); }
}
public Board (int size)
{
_cells = new Player? [size, size];
}
public Player? this [int row, int column] {
get {
return _cells [row, column];
} set {
if (_cells [row, column].HasValue)
throw new InvalidOperationException ("The cell is already claimed.");
_cells [row, column] = value;
}
}
}
IBoardAnalyzer
has a single method that scans the board to find the winner, similarly to yours:
interface IBoardAnalyzer
{
Player? DetermineWinner (IBoard board);
}
All the actual heavy-lifting for scanning the board lives in BoardExtensions
that can enumerate board rows, columns and diagonals. There is also an über-method called SelectAllLines
that returns a sequence of all “lines” (rows, columns and diagonals) on the board. Note that BoardExtensions
does no analysis; it only provides convenience methods for extracting data out of Board
.
static class BoardExtensions
{
public enum DiagonalKind {
Primary,
Secondary
}
public static IEnumerable<IEnumerable<Player?>> SelectAllLines (this IBoard board)
{
return board.SelectAllDiagonals ()
.Concat (board.SelectAllRows ())
.Concat (board.SelectAllColumns ());
}
public static IEnumerable<IEnumerable<Player?>> SelectAllRows (this IBoard board)
{
return from row in board.SelectIndices ()
select board.SelectRow (row);
}
public static IEnumerable<IEnumerable<Player?>> SelectAllColumns (this IBoard board)
{
return from column in board.SelectIndices ()
select board.SelectColumn (column);
}
public static IEnumerable<IEnumerable<Player?>> SelectAllDiagonals (this IBoard board)
{
return from kind in new [] { DiagonalKind.Primary, DiagonalKind.Secondary }
select board.SelectDiagonal (kind);
}
public static IEnumerable<Player?> SelectRow (this IBoard board, int row)
{
return from column in board.SelectIndices ()
select board [row, column];
}
public static IEnumerable<Player?> SelectColumn (this IBoard board, int column)
{
return from row in board.SelectIndices ()
select board [row, column];
}
public static IEnumerable<Player?> SelectDiagonal (this IBoard board, DiagonalKind kind)
{
return from index in board.SelectIndices ()
let row = index
let column = (kind == DiagonalKind.Primary)
? index
: board.Size - 1 - index
select board [row, column];
}
public static IEnumerable<int> SelectIndices (this IBoard board)
{
return Enumerable.Range (0, board.Size);
}
}
The implementation for BoardAnalyzer
uses BoardExtensions
to find the winner, but in itself is trivial:
class BoardAnalyzer : IBoardAnalyzer
{
public Player? DetermineWinner (IBoard board)
{
return (
from line in board.SelectAllLines ()
let winner = DetermineLineWinner (line)
where winner.HasValue
select winner
).FirstOrDefault ();
}
static Player? DetermineLineWinner (IEnumerable<Player?> line)
{
try {
return line.Distinct ().Single ();
} catch (InvalidOperationException) {
return null;
}
}
}
Finally, I implemented an IO which has a simple interface:
interface IGameIO
{
Tuple<int, int> AskNextMove (Player player, IBoard board);
void DisplayError (GameError error);
void DisplayWinner (Player player);
}
And just as simple implementation:
class ConsoleGameIO : IGameIO
{
public Tuple<int, int> AskNextMove (Player player, IBoard board)
{
Console.WriteLine ("\n\n");
Console.WriteLine ("{0}, what is your move?\n", FormatPlayer (player));
Console.WriteLine (FormatBoard (board));
Console.Write ("\nType A1 to C3: ", FormatPlayer (player));
return ParseMove (Console.ReadLine ().Trim ().ToUpperInvariant ());
}
public void DisplayError (GameError error)
{
switch (error) {
case GameError.CellAlreadyOccupied:
Console.WriteLine ("Bad move: cell is already occupied.");
break;
case GameError.CouldNotParseMove:
Console.WriteLine ("Bad move. Valid moves are A1 to C3.");
break;
default:
Console.WriteLine ("Something went wrong.");
break;
}
}
public void DisplayWinner (Player player)
{
Console.WriteLine ("Congatulations, {0}! You won.", FormatPlayer (player));
}
static string FormatBoard (IBoard board)
{
return string.Join ("\n", from row in board.SelectAllRows () select FormatRow (row));
}
static string FormatRow (IEnumerable<Player?> row)
{
return string.Join ("|", from cell in row select FormatCell (cell));
}
static string FormatPlayer (Player player)
{
return FormatCell (player);
}
static string FormatCell (Player? cell)
{
return cell.HasValue ? cell.Value.ToString () : "_";
}
static Tuple<int, int> ParseMove (string input)
{
return Tuple.Create (
input [0] - 'A',
input [1] - '1'
);
}
}
Finally, there goes the Game
class with the run loop and Main
method:
class Game
{
IBoard board;
IBoardAnalyzer analyzer;
IGameIO io;
public Game (IBoard board, IBoardAnalyzer analyzer, IGameIO io)
{
this.board = board;
this.analyzer = analyzer;
this.io = io;
}
public void Run ()
{
Player player = Player.X;
Player? winner = null;
do {
Tuple<int, int> move;
try {
move = io.AskNextMove (player, board);
} catch {
io.DisplayError (GameError.CouldNotParseMove);
continue;
}
if (!ValidateMove (move)) {
io.DisplayError (GameError.CouldNotParseMove);
continue;
}
try {
board [move.Item1, move.Item2] = player;
} catch (InvalidOperationException) {
io.DisplayError (GameError.CellAlreadyOccupied);
continue;
}
player = GetNextPlayer (player);
winner = analyzer.DetermineWinner (board);
} while (!winner.HasValue);
io.DisplayWinner (winner.Value);
}
bool ValidateMove (Tuple<int, int> move)
{
return (move.Item1 >= 0 && move.Item1 < board.Size)
&& (move.Item2 >= 0 && move.Item2 < board.Size);
}
static Player GetNextPlayer (Player player)
{
switch (player) {
case Player.X:
return Player.O;
case Player.O:
return Player.X;
default:
throw new NotImplementedException ();
}
}
public static void Main (string[] args)
{
var game = new Game (new Board (3), new BoardAnalyzer (), new ConsoleGameIO ());
game.Run ();
}
}
Check it out on Github.