Number Guessing Game in Haskell

Question

I've been learning a lot about functional programming lately and I've recently started learning Haskell to which I'm very new.

I've written a simple number guessing game and I would like to have feedback on what is good or bad about my code and how it could be improved.

#!/usr/bin/env runhaskell
import Text.Printf (printf)
import System.Random (randomRIO)
smallest = 1 :: Integer
biggest = 100 :: Integer
main :: IO ()
main = do
 secret <- randomRIO (smallest, biggest) :: IO Integer
 printf "Find the number between %d and %d.\n" smallest biggest
 loop secret 1 where
 loop :: Integer -> Integer -> IO ()
 loop secret tries = do
 guess <- readLn :: IO Integer
 case compare guess secret of
 LT -> do
 putStrLn "Too small!"
 loop secret $ succ tries
 GT -> do
 putStrLn "Too big!"
 loop secret $ succ tries
 EQ -> do
 printf "You found the number in %d tries!\n" tries

• bdecaf: I think it would be good practice to handle non numeric inputs instead of throwing an exception. Maybe even add an input to quit the program.

Answer 1

Changing the where to a let allows you to skip passing the secret. Passing guesses in and messages out purifies the control flow part of the program. zip [1..] makes clear: The number of tries during each step does not depend on the control flow. interact, lines and foldr replace the recursion. (Recursion is too powerful for brains to handle in general, so we should avoid it when we have a simpler tool.) Your mileage may vary on any of these suggestions.

secret <- randomRIO (smallest, biggest) :: IO Integer
let foo :: (Integer, Integer) -> String -> String
 foo (tries, guess) = case compare guess secret of
 LT -> (++) "Too small!\n"
 GT -> (++) "Too big!\n"
 EQ -> const $ printf "You found the number in %d tries!\n" tries
printf "Find the number between %d and %d.\n" smallest biggest
interact $ foldr foo undefined . zip [1..] . map read . lines

Answer 2

Overall, good work. It's a solid first number guessing game.

The good

You're using type annotations. That's great. The type inference actually needs only two annotations (one of secret, smallest or biggest, and tries).

The bad

However, your program fails if the user does not enter a number:

Find the number between 1 and 10.
Hello
*** Exception: user error (Prelude.readIO: no parse)

That's not very user friendly. More on that in the next section.

The ugly

While I don't concur with @Gurkenglas that recursion is too powerful for brains, there are some weird things going on with loop:

• you have to thread secret throughout all recursive calls
• you use succ tries instead of tries + 1

Good code is written for humans and machines, but the latter are second class citizens. So let's try to get rid of some noise:

main :: IO ()
main = do
 secret <- randomRIO (smallest, biggest) :: IO Integer
 printf "Find the number between %d and %d.\n" smallest biggest
 let loop tries :: Int -> IO ()
 loop tries = do
 guess <- askNumberOrQuit
 case compare guess secret of
 LT -> do
 putStrLn "Too small!"
 loop (tries + 1)
 GT -> do
 putStrLn "Too big!"
 loop (tries + 1)
 EQ -> do
 printf "You found the number in %d tries!\n" tries
 loop

The function askNumberOrQuit is left as an exercise, but for a first implementation you can think of it as

askNumberOrQuit :: IO Integer
askNumberOrQuit = readLn

There are more elegant ways to handle this, e.g. prompt the user again if they didn't write a number, or quit if the user writes q[uit].

Either way, loop is now a lot larger than the original main. And since we have to indent it with let, there's a lot of whitespace on the left hand side, usually a sign that we should move things.

So let us go ahead and move loop out of main:

-- | Asks the user for a number until
-- they guessed the correct one.
guessNumber :: Integer -> IO ()
guessNumber secret = go 1 
 where
 go tries = do
 guess <- askNumberOrQuit
 case compare guess secret of
 LT -> putStrLn "Too small!" >> go (tries + 1)
 GT -> putStrLn "Too big!" >> go (tries + 1)
 EQ -> printf "You found the number in %d tries!\n" tries

Using >> instead of do is a personal choice, but I like that both go (tries + 1) line up. YMMV.

This brings us to the following succinct main:

module Main (main) where
main :: IO ()
main = do
 printf "Find the number between %d and %d.\n" smallest biggest
 secret <- randomRIO (smallest, biggest) :: IO Integer
 guessNumber secret

Note that I've moved secret closer to its point of use. This motivates

main :: IO ()
main = do
 printf "Find the number between %d and %d.\n" smallest biggest
 randomRIO (smallest, biggest) >>= guessNumber

But again, that's a matter of choice and up to you. So what's the point of main being so small? Well, at the moment, there's no choice for the user to make the game harder or easier. Adding that to your "large" main above would make things messy. Adding this to the small main makes things not-messy:

-- | Returns a ordered pair from user input,
-- or default range if the user enters "default".
askForRangeOrDefault :: IO (Integer, Integer)
main :: IO ()
main = do
 putStrLn "Welcome to the number guesser!"
 (smallest, biggest) <- askForRangeOrDefault
 printf "Find the number between %d and %d.\n" smallest biggest
 randomRIO (smallest, biggest) >>= guessNumber

The nice part is that regardless of how you mess up main, guessNumber will still work. That's the great part of modularity.