patterncsharpMinor
Check winner in a Tic Tac Toe game
Viewed 0 times
toetictacgamewinnercheck
Problem
I have the below code to check for a winner in a Tic Tac Toe game. I'm wondering if this is a good approach, and if there is a better way of doing this (maybe by monitoring the state of the board).
public void CheckWinner()
{
var board = GameBoard.Content;//stores data as dictionary
for (int i = 0; i , bool> containsO = input => (input.Key == a || input.Key == b || input.Key == c) && input.Value == "O";
Func, bool> containsX = input => (input.Key == a || input.Key == b || input.Key == c) && input.Value == "X";
var oResult = board.Where(containsO).ToDictionary();
if (oResult.Count == 3)
{
Winner = Player.Computer;
WinnerIndex = oResult.Keys.ToArray();
_gameOver = true;
}
var xResult = board.Where(containsX).ToDictionary();
if (xResult.Count == 3)
{
Winner = Player.Human;
WinnerIndex = xResult.Keys.ToArray();
_gameOver = true;
}
}
}
if (board.Count == 9)
{
_gameOver = true;
Winner = null;
}
}Solution
General advice:
-
Use enums instead of strings for board squares. This will protect you against mistakes like
-
A dictionary is overkill for a 3x3 board, you can just use an array. One benefit is that a bad index will throw an
-
Board logic should be encapsulated in a class, so that you can make changes to your implementation (like switching from a dictionary to an array) without the rest of your code having to change. It will also allow you to guard against errors such as overwriting a non-empty board square.
To make the function easier to test:
-
Pass the board to the function, or make this a method of your Board class, instead of referring to
-
Return an object representing the game state, instead of modifying private fields.
Together, it might look like this:
-
Use enums instead of strings for board squares. This will protect you against mistakes like
board[i] = "x".-
A dictionary is overkill for a 3x3 board, you can just use an array. One benefit is that a bad index will throw an
IndexOutOfRangeException, while a dictionary will not complain.-
Board logic should be encapsulated in a class, so that you can make changes to your implementation (like switching from a dictionary to an array) without the rest of your code having to change. It will also allow you to guard against errors such as overwriting a non-empty board square.
To make the function easier to test:
-
Pass the board to the function, or make this a method of your Board class, instead of referring to
GameBoard.Content.-
Return an object representing the game state, instead of modifying private fields.
Together, it might look like this:
public static GameState GetGameState(Board board)
{
foreach (var winningLine in WinningLines)
{
var line = new BoardState[]
{
board[winningLine[0]],
board[winningLine[1]],
board[winningLine[2]]
};
if (line.All(state => state == BoardState.Human))
{
return GameState.HumanWon(winningLine);
}
if (line.All(state => state == BoardState.Computer))
{
return GameState.ComputerWon(winningLine);
}
}
return board.Any(state => state == BoardState.Empty)
? GameState.Unfinished
: GameState.Tie;
}Code Snippets
public static GameState GetGameState(Board board)
{
foreach (var winningLine in WinningLines)
{
var line = new BoardState[]
{
board[winningLine[0]],
board[winningLine[1]],
board[winningLine[2]]
};
if (line.All(state => state == BoardState.Human))
{
return GameState.HumanWon(winningLine);
}
if (line.All(state => state == BoardState.Computer))
{
return GameState.ComputerWon(winningLine);
}
}
return board.Any(state => state == BoardState.Empty)
? GameState.Unfinished
: GameState.Tie;
}Context
StackExchange Code Review Q#53841, answer score: 8
Revisions (0)
No revisions yet.