3
\$\begingroup\$

I'm beginning (for the third time in 5 years) to learn Haskell and decided to do a "More or less" console game (game pick a random number, you then have to guess which).

First the code, then my question :

import System.Random  
import Text.Read(readMaybe)

main :: IO ()
main = do 
  gen <- getStdGen
  let maxSecret = 1000
  let (secret, newGen) = randomR (1,maxSecret) gen :: (Int, StdGen)

  -- putStrLn $ show secret
  let oracle = compareToSecret secret
  askForNumber oracle 1 (1, maxSecret)


data Guess = SecretIsLess | SecretIsMore | Win deriving (Show)

askForNumber :: (Int -> Guess) -> Int -> (Int, Int) -> IO ()
askForNumber oracle tries range = do
  let (min, max) = range
  putStrLn $ "Guess the number between " ++ (show min) ++ " and " ++ (show max) ++ " !"
  guessInput <- getLine
  let guess = readMaybe guessInput :: Maybe Int

  case (oracle) <$> guess of 
            Just SecretIsMore -> do
                  putStrLn "Guess more !"
                  askForNumber oracle (tries+1) (maybe min id guess, max)
            Just SecretIsLess -> do
                  putStrLn "Guess less !"
                  askForNumber oracle (tries+1) (min, maybe max id guess)
            Just Win -> do
                  putStrLn "You win !"
                  putStrLn $ "It took you " ++ (show tries) ++ " tries."
                  return ()
            Nothing -> do
                  putStrLn "That is not a number"
                  askForNumber oracle tries range
    
compareToSecret :: Int -> Int -> Guess
compareToSecret secret guess
  | secret > guess = SecretIsMore
  | secret < guess = SecretIsLess
  | secret == guess = Win

I'd like to improve that code to make it idiomatic and learn in the process.

I made a choice : at first I had
compareToSecret :: Int -> Int -> (Guess, Int)
and in the case of I had something like
Just (SecretIsMore, newmin) -> do askForNumber oracle (tries+1) (newmin, max)

Do you think it's better to kind of unpack with maybe there or to pass the value around and keep it pure ? Or is there another solution ?

\$\endgroup\$
3
\$\begingroup\$

Prelude

I think your code (sans bug) is fine. Your question is already a little nit-picky so my comments will mostly be nit-picks. I am not a Haskell expert either, so don't place too much authority in my comments. Evaluate them for yourself.

My comments are in a somewhat arbitrary order, but I've organized them with headers. The section pertaining specifically to your question is titled "What to do with the case".

The revised code is included at the end.

The Guess datatype

A datatype that is used precisely like this already exists, it's called Ordering and can be found in Data.Ord (although I believe it's included by default). No need to reinvent the wheel here. Especially because you'll see later that you can use already-made functions that work with Ordering.

Input validation

You correctly check that the input is a well-formed number, but you don't check that it's between the min and max value of the range. Here's an example of how this bug manifests itself.

Guess the number between 1 and 1000 !
50
Guess more !
Guess the number between 50 and 1000 !
20
Guess more !
Guess the number between 20 and 1000 !

Notice how the value goes down. The fix, which I will put in the section addressing your questions about Maybe, is to add another case that deals with a well-formed, but invalid guess.

Syntax and other small details

min and max are built-in functions, it's best not to shadow them. I would call them minValue and maxValue.

You can pattern match in function definitions so that you don't need to unpack range – you can instead have

askForNumber oracle tries (minValue, maxValue) = do
...

Function application has the highest precedence in Haskell, so you don't need parens around (show min). (oracle) <$> guess doesn't need the parens either.

maybe value id = fromMaybe value, so use that instead. You can get fromMaybe from Data.Maybe, but it might be included by default.

oracle

If you switch to using Ordering, you can nix your compareToSecret function and define oracle = compare secret.

What to do with the case

I think it's kind of clunky to extract a value from a Maybe after you've cased on it, which is what you're doing right now. A big utility of case is knowing that your value must be of a certain form in each branch. In that branch, guess must be of the form Just _. While you as a programmer know this to be true, it's much more helpful to have the type system be able to prove this.

As I see it, there are two ways to handle your case statement.

The first is to have two nested case statements. The first branches on guess. If guess is Nothing, it prompts the user for another input. If guess is Just g, it cases on oracle g.

The second is to do a slight modification of what you're doing. Instead of fmapping oracle, you can fmap the function \g -> (oracle g, g). This allows you to extract the guess safely without having to use Maybe. I believe this is what you were doing originally.

I prefer to avoid nesting cases when I can, so I opted for the second one.

If you would also like to validate the input (I chose to), then you will want to add a case at the beginning matching Just (_, guess) – i.e. one that ignores the comparison – to deal with invalid guesses. This pattern, however, will match both invalid and valid guesses. We need to add a guard that asserts that guess > max or guess < min. This looks like:

    ...
    Just (_, guess) | guess < min || guess > max -> do
      putStrLn "Guess out of range"
      askForNumber oracle tries range
    ...

Small note on using oracle

I use the function \g -> (oracle g, g) which might feel slightly clunky. Indeed, there are a few alternatives, but I warn you that they are somewhat arcane. In general, I would avoid using "cute" or "slick" code like this unless you know everyone who sets eyes on the code will understand it.

All of the below are equivalent (for your use case):

\g -> (oracle g, g)    -- plain definition
oracle &&& id          -- (&&&) needs to be imported from Control.Arrow
(,) =<< oracle         -- using the Reader Monad
(,) <$> oracle <*> id  -- using the Reader Applicative

I would be most inclined to use oracle &&& id because it'd be the most recognizable, but even it is a little arcane. I pretty much only included the last example because you'll see code that looks like f <$> g <*> h <*> i … a lot; it's a common idiom for other Applicatives (but I would advise against using it in this case). No matter what people say or do, there's nothing wrong with writing "plain" or "simple" Haskell.

Revised code

You'll find that not too much is different.

import System.Random  
import Text.Read (readMaybe)

main :: IO ()
main = do 
  gen <- getStdGen
  let maxSecret = 1000
  let (secret, newGen) = randomR (1,maxSecret) gen :: (Int, StdGen)

  -- putStrLn $ show secret
  let oracle = compare secret
  askForNumber oracle 1 (1, maxSecret)

askForNumber :: (Int -> Ordering) -> Int -> (Int, Int) -> IO ()
askForNumber oracle tries range@(minValue, maxValue) = do
  putStrLn $ "Guess the number between " ++ show minValue ++ " and " ++ show maxValue ++ " !"
  guessInput <- getLine
  let guessMaybe = readMaybe guessInput :: Maybe Int

  case (\guess -> (oracle guess, guess)) <$> guessMaybe of 
    Nothing -> do
      putStrLn "That is not a number"
      askForNumber oracle tries range
    Just (_, guess) | guess < minValue || guess > maxValue -> do
      putStrLn "Guess out of range"
      askForNumber oracle tries range
    Just (LT, guess) -> do
      putStrLn "Guess less !"
      askForNumber oracle (tries+1) (minValue, guess)
    Just (GT, guess) -> do
      putStrLn "Guess more !"
      askForNumber oracle (tries+1) (guess, maxValue)
    Just (EQ, _) -> do
      putStrLn "You win !"
      putStrLn $ "It took you " ++ (show tries) ++ " tries."
```
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks a lot ! In my first version I had indeed two nested cases, one dealing with the user error and the other inside the Just match but that was clunky to edit since you have to respect indentation and I spent more time reindenting than actually thinking :) Your answer regarding the "maybe unpacking" is the perfect middle ground between the two options I thought of ! (and I learned in the proces) \$\endgroup\$ – smwhr Jul 10 at 4:39
  • \$\begingroup\$ Glad to help; you’ll find that casing on tuples like this, though seemingly clunky at first, oftentimes cleans up the code a decent deal. Especially when you reach the point in a code base where you have to have a lot of nested cases. \$\endgroup\$ – cole Jul 10 at 8:16

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.