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

Adding more responsibility to this endgame-analyzer class

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

Problem

I have written a Tic-Tac-Toe game. I have a class called GameEndAnalyzer that will check if the game has already ended or not.

Would it violate the Single Responsibility Principle if I will add more responsibilities to this class to not only decide if someone has won the game, but who has won (X or O)? What is the best way to add responsibility to this?

```
public interface IGameEndAnalyzer
{
bool IsGameEnd(IBoard board);
}
public class GameEndAnalyzer : IGameEndAnalyzer
{
public bool IsGameEnd(IBoard board)
{
return IsAnyRowHasAllSameValue(board) ||
IsAnyColumnHasAllSameValue(board) ||
IsLeftDiagonalHasAllSameValue(board) ||
IsRightDiagonalHasAllSameValue(board);
}

private bool IsAnyRowHasAllSameValue(IBoard board)
{
for (int rowIndex = Board.Row.Top; rowIndex <= Board.Row.Bottom; rowIndex++)
{
var isSameValue = board
.Cells
.WhereInRow(rowIndex)
.AllCellIs_O_Or_X();

if (isSameValue)
{
return true;
}
}

return false;
}

private bool IsAnyColumnHasAllSameValue(IBoard board)
{
for (int columnIndex = Board.Column.Left; columnIndex <= Board.Column.Right; columnIndex++)
{
var isSameValue = board
.Cells
.WhereInRow(columnIndex)
.AllCellIs_O_Or_X();

if (isSameValue)
{
return true;
}
}

return false;
}

private bool IsLeftDiagonalHasAllSameValue(IBoard board)
{
var isSameValue = board
.Cells
.WhereInDiagonalLeft()
.AllCellIs_O_Or_X();

return isSameValue;
}

private bool IsRightDiagonalHasAllSameValue(IBoard board)
{

Solution

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 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> SelectAllLines (this IBoard board)
    {
        return board.SelectAllDiagonals ()
            .Concat (board.SelectAllRows ())
            .Concat (board.SelectAllColumns ());
    }

    public static IEnumerable> SelectAllRows (this IBoard board)
    {
        return from row in board.SelectIndices ()
               select board.SelectRow (row);
    }

    public static IEnumerable> SelectAllColumns (this IBoard board)
    {
        return from column in board.SelectIndices ()
               select board.SelectColumn (column);
    }

    public static IEnumerable> SelectAllDiagonals (this IBoard board)
    {
        return from kind in new [] { DiagonalKind.Primary, DiagonalKind.Secondary }
               select board.SelectDiagonal (kind);
    }

    public static IEnumerable SelectRow (this IBoard board, int row)
    {
        return from column in board.SelectIndices ()
               select board [row, column];
    }

    public static IEnumerable SelectColumn (this IBoard board, int column)
    {
        return from row in board.SelectIndices ()
               select board [row, column];
    }

    public static IEnumerable 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 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 line)
    {
        try {
            return line.Distinct ().Single ();
        } catch (InvalidOperationException) {
            return null;
        }
    }
}


-
Finally, I implemented an IO which has a simple interface:

```
interface IGameIO
{
Tuple AskNextMove (Player player, IBoar

Code Snippets

enum Player {
    X,
    O
}
interface IBoard
{
    int Size { get; }
    Player? this [int row, int column] { get; set; }
}
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;
        }
    }
}
interface IBoardAnalyzer
{
    Player? DetermineWinner (IBoard 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);
    }
}

Context

StackExchange Code Review Q#38632, answer score: 6

Revisions (0)

No revisions yet.