Skip to content

Commit

Permalink
factor check for move validity into Board.Apply
Browse files Browse the repository at this point in the history
It feels more natural and less error-prone to have the Board.Apply
method check whether the given move is valid, instead of making it
a precondition and counting on the caller to do that.

This makes it slightly harder to distinguish whether Board.Apply
failed because the move was invalid or illegal, but that can be
resolved with better errors if it becomes important.

GitHub-Pull-Request: #2
  • Loading branch information
quackduck committed Jun 13, 2021
1 parent e4032f5 commit d19fb9e
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
5 changes: 1 addition & 4 deletions cmd/tictactoe/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,10 @@ func playerTurn(b *ttt.Board, player player, cellClick <-chan int) error {
if err != nil {
return fmt.Errorf("player %v (%s) failed to make a move: %v", player.Mark, player.Name(), err)
}
if err := move.Valid(); err != nil {
return fmt.Errorf("player %v (%s) made a move that isn't valid: %v", player.Mark, player.Name(), err)
}

err = b.Apply(move, player.Mark)
if err != nil {
return fmt.Errorf("player %v (%s) made a move that isn't legal: %v", player.Mark, player.Name(), err)
return fmt.Errorf("player %v (%s) made a move that isn't legal or isn't valid: %v", player.Mark, player.Name(), err)
}

return nil
Expand Down
7 changes: 5 additions & 2 deletions tictactoe.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,12 @@ type Board struct {
Cells [9]State
}

// Apply a valid move to this board. Mark is either X or O.
// If it's not a legal move, the board is not modified and the error is returned.
// Apply a move to this board. Mark is either X or O.
// If it's not a legal, valid move, the board is not modified and an error is returned.
func (b *Board) Apply(move Move, mark State) error {
if err := move.Valid(); err != nil {
return err
}
// Check if the move is legal for this board configuration.
if b.Cells[move] != F {
return fmt.Errorf("that cell is already occupied")
Expand Down

0 comments on commit d19fb9e

Please sign in to comment.