Basic Nim game in Haskell

Question

I am starting to learn Haskell, and following Dr. Erik Meijer lectures online, he suggests as an exercise to implement the game of Nim in Haskell, so I implemented a really basic game for two players (no computer player).

I can imagine that this code is far from being good Haskell, and I accept all possible criticism and suggestions about it (if you can link me to other lectures/talks/guidelines, I appreciate it), but there are some aspects that I would love to ask you:

1. The comments: I am not really sure how am I supposed to document the code in Haskell, so I just tried to explain each of the methods before its declaration. If there is another convention, please let me know.

2. The parenthesis: Should I use more the $ or . operators than parenthesis? I kind of struggle with trying to use composition with functions when there is more than one argument.

3. The nested do blocks: I fixed an error doing it, but it feels incredibly hacky and I don't really understand why the compiler is not able to understand the code when there is more than one line inside an if clause (doesn't matter which one, both were problematic). If I don't put the do keyword, the compiler was interpreting that putStrLine was receiving 4 arguments: its real argument and the next function call (turn board player, for instance). Why did it happen?

This is the code itself:

import qualified Data.Sequence as Seq
import qualified Data.Foldable as Fol
import qualified Data.List as List
import Data.Maybe

-- Player type is used to take control of the turns and who wins
data Player = One | Two deriving (Show, Eq)

-- Change player returns the player that is NOT the one passed as argument
change :: Player -> Player
change One = Two
change Two = One

-- Board is an alias for a Sequence of Ints, as they allow us to update single
-- elements in easily.
type Board = Seq.Seq Int

-- This is the initial board structure
initialBoard :: Board
initialBoard = Seq.fromList [5, 4, 3, 2, 1]

-- The move method checks if the a movement can be executed and returns the
-- updated board in case it is possible
move :: Board -> (Int, Int) -> Maybe Board
move board (row, stars)
  | and [(Seq.index board row) >= stars,
          row < 5] = Just (Seq.adjust (\x -> x - stars) row board)
  | otherwise = Nothing

-- The display methods transforms a Board into a nice, enumerated String of
-- asterisks
display :: Board -> String
display board = List.intercalate "\n" (zipWith (++) numbers (stars board))
                where numbers = ["1. ", "2. ", "3. ", "4. ", "5. "]
                      stars board = [(concat . take n) (repeat "* ")
                                    | n <- Fol.toList board]

-- The next methods are the ones that control IO
main :: IO ()
main = nim

-- Main method welcomes the player, displays the initial board and calls the
-- first turn
nim :: IO ()
nim = do putStrLn "Welcome to nim!"
         putStrLn (display initialBoard)
         turn initialBoard One

-- The turn method displays the player and asks for a movement, then checks if
-- there was a problem performing that movement and continues the game. This is
-- the main game loop
turn :: Board -> Player -> IO ()
turn board player = do putStrLn ("\nIt's your turn, Player " ++ (show player) ++ "!")
                       putStrLn "Choose a row to remove stars!"
                       row <- getLine
                       putStrLn "How many stars do you want to remove?"
                       stars <- getLine
                       let newBoard = move board ((read row) - 1, read stars)
                       if newBoard == Nothing
                         then do putStrLn "Not valid movement"
                                 turn board player
                         else isOver (fromJust newBoard) (change player)

-- isOver checks if the Board is empty, and checks whether the game is over or
-- the next turn must be called
isOver :: Board -> Player -> IO()
isOver board player = do if board == Seq.fromList [0, 0, 0, 0, 0]
                           then putStrLn ("Congratulations, Player " ++ (show player)
                                         ++ ", you win!")
                           else do putStrLn ""
                                   putStrLn (display board)
                                   turn board player

Answer 1

1. Haskell generates rich documentation from specially marked up comments using Haddock. If you're finding it difficult to write a description that does more than restate the type give some usage examples. E.g.,

   >>> change One
   Two
   

   Then consider future proofing your examples with doctest.

2. This is fine, don't worry about it.

3. (This explanation trades rigor for accessibility. Hopefully.)

   The contents of the then and else blocks aren't part of the overall turn do-block. The if statement returns whichever of then or else based on the given predicate, which in your case must be an IO monad action, but in the general case of ifs doesn't have to be.

   While if/then/else is syntax, consider this function definition which is equivalent and not syntax.

   boolCase :: Bool -> a -> a -> a
   boolCase True  a _ = a
   boolCase False _ b = b
   

   So your broken if statement was the same as—

   boolCase (newBoard == Nothing)
            (putStrLn "Not valid movement"
             turn board player)
            (isOver (fromJust newBoard) (changePlayer))
   

   Now that it looks like a normal function call do you see the problem with the second argument to boolCase? Without explicitly using do you are calling putStrLn with four arguments as that line break is no longer meaningful. This is the same as when you use if/then/else. Just because you are using an if in a do-block doesn't mean that the contents of then and else are in that same do-block.

---

Use type aliases to disambiguate repeated types that would be inconvenient to newtype. This improves your documentation without needing to write boring comments too!

type Row = Int
type Stars = Int

Don't tuple your arguments to functions unnecessarily. E.g., use move :: Board -> Row -> Stars -> Maybe Board instead of what you have now.

move has an error in that it checks the number of stars in a row before it checks that the row is in bounds.

move is kinda hairy with that multi-line guard. Here's an alternate version (one of many possible!) that takes advantage of the Alternative instance of Maybe (and should take advantage of the new ApplicativeDo extension if you enable it!).

import Control.Monad (guard)

move :: Board -> Row -> Stars -> Maybe Board
move board row stars = do
    guard $ 0 <= row && row < 5
    guard $ stars <= board `Seq.index` row
    pure $ Seq.adjust (subtract stars) row board

Here's an alternate version of display too. Take advantage of pre-existing functions with semantically meaningful names!

import Data.List (intersperse, replicate)

display :: Board -> String
display board = unlines $ zipWith (++) numbers (stars board)
  where
    numbers = ["1. ", "2. ", "3. ", "4. ", "5. "]
    stars = map (intersperse ' ' . flip replicate '*') . toList

Your game play and display logic is smeared across nim, turn and isOver. Try extracting the pure components into reusable, nameable functions (like the proposition in isOver). Instead of if and then fromJust, use a case statement (case newBoard of Nothing -> ...; Just b -> ...). Use a safe version of read (Text.Read.readMaybe) and deal with errors properly.

You might also want to use Data.Array instead of Data.Sequence for constant time indexing.

With practice and familiarity I think you'll do fine! This is really very good for a beginner.