5
\$\begingroup\$

I've created a two-player number guessing game in Haskell. My main objective was to practice dealing with "state" in a purely functional language (such as player scores, whose turn it is, etc.).

Here are the rules:

Gameplay:

Two players will take turns guessing a random number between 1 and 10. Answers will be typed into the command line.

Scoring:

  • If a player guesses the number correctly, they will be awarded 5 points
  • If a player is within two (inclusive) from the answer, the player will be awarded 3 points.
  • If a player is within three (inclusive) from the answer, they will be awarded 1 point.
  • If a player is 7 or more points off, they will lose a point. The score may not be negative.
  • All other offsets will result in zero points.
  • The game will continue until the one of the players reaches 10 points.

Caveats:

  1. This is was not designed to be an exercise in enjoyable game design -- obviously the optimal solution is to always choose five, which doesn't make for a lot of excitement. :D
  2. I am aware that Control.Monad.State exists, but I want to practice tracking state without it.
  3. I know that the "mutual recursion" is difficult to follow. I would love some suggestions for getting rid of that which do not involve nesting if statements.
import Data.Char
import System.Random

main = do
    stdGen <- getStdGen
    play 0 0 P1 stdGen

play :: Int -> Int -> Player -> StdGen -> IO ()
play p1Score p2Score player stdGen
    | p1Score < 10 && p2Score < 10 = continueGame p1Score p2Score player stdGen
    | otherwise = putStrLn $ show (determineWinner p1Score p2Score) ++ " wins!"

continueGame :: Int -> Int -> Player -> StdGen -> IO ()
continueGame p1Score p2Score player stdGen = do
    putStr $ show player ++ "'s turn. Pick a number between 1 and 10: "
    chosenNumber <- getLine
    if isInteger chosenNumber
        then do
            let (randomNumber, newGen) = randomR (1, 10) stdGen :: (Int, StdGen)
            putStrLn $ "The answer is " ++ show randomNumber

            let pointsEarned = calcPointsEarned randomNumber (read chosenNumber)
            let newP1Score = min (max (p1Score + calcPointsEarnedForPlayer player P1 pointsEarned) 0) 10
            let newP2Score = min (max (p2Score + calcPointsEarnedForPlayer player P2 pointsEarned) 0) 10

            putStrLn $ "P1 Score: " ++ show newP1Score
            putStrLn $ "P2 Score: " ++ show newP2Score

            play newP1Score newP2Score (changeTurn player) newGen
        else do
            putStrLn "The input must be an integer"
            play p1Score p2Score player stdGen

data Player = P1 | P2 deriving (Show, Eq)

isInteger :: String -> Bool
isInteger = and . map isNumber

changeTurn :: Player -> Player
changeTurn player 
    | player == P1 = P2
    | otherwise = P1

calcPointsEarned :: Int -> Int -> Int
calcPointsEarned actualAnswer chosenAnswer
    | offset == 0 = 5
    | offset <= 2 = 3
    | offset <= 3 = 1
    | offset >= 7 = (-1)
    | otherwise = 0
    where offset = abs $ chosenAnswer - actualAnswer

calcPointsEarnedForPlayer :: Player -> Player -> Int -> Int
calcPointsEarnedForPlayer actualTurn player pointsEarned 
    | actualTurn == player = pointsEarned
    | otherwise = 0

determineWinner :: Int -> Int -> Player
determineWinner p1Score p2Score
    | p1Score > p2Score = P1
    | otherwise = P2
```
\$\endgroup\$
3
\$\begingroup\$

I'd contend that State is pure and functional, but I think translating your current code to use State is an excellent exercise so I'll leave that up to you.

The first thing I'd address is making your types do more of the bookkeeping. Well designed types lend themselves to correct-by-construction solutions.

data Player = P1 | P2 deriving Show

data Game = Game { turn :: Player, p1 :: Int, p2 :: Int } deriving Show

Prefer pattern matching to equality testing, it is frequently more terse. Decreased line noise often means increased readability.

changeTurn :: Player -> Player
changeTurn P1 = P2
changeTurn P2 = P1

Your determineWinner function has a (currently unreachable) logic error. If both player's scores are equal then it prefers handing victory to player two. This may not matter in your code as written, but if your code changes or you begin property testing or some other unforeseeable future event comes to pass, it could begin mattering unexpectedly. Handling ties is “morally” the right thing to do.

Also as-is it isn't really determining a winner by the rules of the game, only which player's score is higher.

winner :: Game -> Maybe Player
winner (Game _ p1 p2) =
    case (max p1 p2 >= 10, p1 > p2, p2 > p1) of
      (True, True, False) -> Just P1
      (True, False, True) -> Just P2
      (_, _ , _)          -> Nothing

Don't validate and then parse, parse and allow for failure. If your validation code is separate from your parsing code you risk them drifting out of sync and causing errors. In this case, use Text.Read.readMaybe from base and leave out your isInteger function entirely.

It's a good idea to separate as much of your pure game logic from IO actions as possible. It's easier to test, easier to understand, and enables you to reuse functionality you otherwise couldn't.

updateRound :: Int -> Game -> Game
updateRound n (Game P1 p1 p2) = Game P2 (boundScore $ p1 + n) p2
updateRound n (Game P2 p1 p2) = Game P1 p1 (boundScore $ p2 + n)

clamp :: Ord a => a -> a -> a -> a
clamp lo val hi = lo `max` val `min` hi

boundScore :: Int -> Int
boundScore n = clamp 0 n 10

This also obviates the need for changeTurn.

It's also usually handy to separate your display logic from your control logic, even if both are IO actions. It might be useful to you if you make use of the REPL while developing, your types often shouldn't include multiple copies of the same information (e.g., two numbers and their difference) as that carries a risk of the values getting out of sync. Those derived values might be all you want to see while working though.

displayGame :: Game -> IO ()
displayGame (Game _ p1 p2) = do
    putStrLn $ "P1 Score: " ++ show p1
    putStrLn $ "P2 Score: " ++ show p2

That said, it's good to separate your control logic from your sources of input and output also. It makes your program testable without needing to muck about with piping stdin and stdout. There's also a very elegant transformation when you decide to begin using State, but I'll leave that to you to figure out.

gameRound :: Int -> Int -> Game -> (Maybe Player, Game)
gameRound guess answer game =
  let
    score = calcPointsEarned guess answer
    nextGame = updateRound score game
  in
    (winner nextGame, nextGame)

receiveGuess :: Player -> IO Int
receiveGuess player = do
    putStr $ show player ++ "'s turn. Pick a number between 1 and 10: "
    input <- getLine
    case readMaybe input of
      Nothing -> do
        putStrLn "The input must be an integer"
        receiveGuess player
      Just guess -> pure guess

play :: Game -> IO ()
play game = do
    answer <- randomRIO (1, 10)
    guess <- receiveGuess (turn game)
    putStrLn $ "The answer is " ++ show answer
    let (mPlayer, nextGame) = gameRound guess answer game
    displayGame nextGame
    case mPlayer of
      Just player -> putStrLn $ show player ++ " wins!"
      Nothing -> play nextGame
\$\endgroup\$
2
  • 1
    \$\begingroup\$ So much good advice here! I especially like your point about separating IO and control logic -- combining them diminishes the benefits of using a purely functional language. Also, I took your advice to create the Game abstraction, and suddenly my type signatures are lot easy easier to understand. Thanks for the detailed response, it will be enough to keep me busy for a while :) \$\endgroup\$ – cgoates Mar 13 at 7:15
  • \$\begingroup\$ Glad I could help! \$\endgroup\$ – bisserlis Mar 14 at 17:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.