Go Fish game written in Go

A couple months back I made a go fish game in Go, just for the sake of that pun. It was pretty poorly done and made experienced Go-ers cry when they looked at it. I rewrote some of it to be more standard, but I'd still like critiques from the 95% of the world with more Go experience than me.

For those unaware, Go Fish is a children's game where each player has a hand of cards and takes turns calling out a card value, if anyone else has that card they have to give it to them, your turn ends when noone has a card that you call out, and you have to draw a card from the deck before the next person takes their turn. The game ends when there are no more cards in the deck and the winner is whoever gets the most four-of-a-kinds during the game.

```
package main

import (
"fmt"
"math/rand"
"sort"
"time"
)

var cards = [13]string{"2", "3", "4", "5", "6", "7", "8", "9",
"10", "J", "Q", "K", "A"}

//GoFishGame Stores the game state.
type GoFishGame struct {
hands [][]string
deck []string
turn int
scores []int
}

// checkForBooks looks for fours of a kind
// and scores them if we have them.
// If we run out of cards, we draw more.
func (gm *GoFishGame) checkForBooks() {
sort.Strings(gm.hands[gm.turn])
prev := ""
count := 1
for _, card := range gm.hands[gm.turn] {
if card == prev {
count++
if count == 4 {
fmt.Printf("Book of %s.\n", card)
gm.stealCards(card, gm.turn)
gm.scores[gm.turn]++
if gm.isHandEmpty() {
gm.drawCard()
}
}
} else {
count = 1
}
prev = card
}
}

// drawCard takes a card from the deck
// adding it to the current player's hand.
func (gm *GoFishGame) drawCard() {
if !gm.isDeckEmpty() {
card := gm.deck[0]
gm.deck = gm.deck[1:]
if gm.turn == 0 {
fmt.Printf("You drew a %s.\n", card)

The following comments come from reading the code for the first time, straight from top to bottom.

Directly after the definition for the GoFishGame type there should be a method NewGoFishGame that initializes this struct, so that the reader learns some basic assumptions of the game, such as the number of players or how many cards each hand holds.

In the moment that gm.checkForBooks calls gm.stealCards, you are iterating over the current player's hand. At the same time, the stealCards method probably removes some cards from the player's hand. Then, iteration continues.
At this point, it is hard to predict what happens. The good thing is that a player can only ever have one book at a time, so for this particular game, there is no chance of missing a second book in the hand. To be safe, you should return from the function at the end of the if count == 4 { ... } clause.

In the drawCard method, the gm.turn == 0 condition surprised me. At this point I concluded that player 0 is a human player and that players 1 to n are computer players. If so, the code is ok.

In the endPly method, the gameOver variable is not necessary. When you step through the code during debugging, you will know the returned value based on the next line the debugger steps to.

In the getPickComputer method I was surprised again. Up to now I thought that this was a game for 2 to 6 players. But since the computer is fixed to player number 1, it is only a 2 player game. That's unfortunate.

In the documentation of getPickComputer, the word we/our is confusing. I had taken the perspective of the human player, so I thought the computer player would spy on my (the human player's) hand. That would have been mean.

In the makeDeck function, don't seed the random number generator, since it will be predictable. Rather, seed it when starting the whole program. Sure, this program is just a children's game, not a crypto library, but who knows what you will develop next.

At the first look, the makeDeck function didn't seem to shuffle the deck, since in the for loop, you are only using a single variable. The loop should be for indx, tVal := range perm { ... }.

In the opponentHas method, you are dealing with two different cards, one is the requested card and the other is the opponent's card. Neither of them should go by the simple name card. I would rename find to requestedCard and then rename card to opponentCard.

In the playerTurn method, the expression (gm.turn + 1) % 2 appears for the second time. You might consider extracting this into its own method, which will become helpful when you expand the game for 6 players later. But even if you don't extract the code, you would have to look at all the code accessing gm.turn, which would also work.

Still in the playerTurn method, the gameOver variable is not necessary, as above.

Still in the playerTurn method, after appending a card to a hand I would expect that hand to be sorted. When you do this, the checkForBooks and printHand methods don't have to do the sorting anymore, and you are guaranteeing that all hands are always sorted.

In the main function, you don't need the variables playerHand and computerHand, although it doesn't hurt. You can initialize the scores using a single line: scores := []int{0, 0}. Same for the hands. All this code should go into the NewGoFishGame function, so that the main function only creates a new game and then calls the method play on it. It sounds weird that the method playerTurn is the whole game; I would have expected it to just execute a single move.

StackExchange Code Review Q#91602, answer score: 5