patternMinor
Basic Nim game in Haskell
Viewed 0 times
gamenimbasichaskell
Problem
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:
-
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.
-
The parenthesis: Should I use more the
-
The nested
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]
-- T
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:
-
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.
-
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.-
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]
-- T
Solution
-
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.,
Then consider future proofing your examples with doctest.
-
This is fine, don't worry about it.
-
(This explanation trades rigor for accessibility. Hopefully.)
The contents of the
While
So your broken
Now that it looks like a normal function call do you see the problem with the second argument to
Use type aliases to disambiguate repeated types that would be inconvenient to
Don't tuple your arguments to functions unnecessarily. E.g., use
Here's an alternate version of
Your game play and display logic is smeared across
You might also want to use
With practice and familiarity I think you'll do fine! This is really very good for a beginner.
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
TwoThen consider future proofing your examples with doctest.
-
This is fine, don't worry about it.
-
(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 = bSo 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 = IntDon'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 boardHere'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 '*') . toListYour 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.
Code Snippets
>>> change One
TwoboolCase :: Bool -> a -> a -> a
boolCase True a _ = a
boolCase False _ b = bboolCase (newBoard == Nothing)
(putStrLn "Not valid movement"
turn board player)
(isOver (fromJust newBoard) (changePlayer))type Row = Int
type Stars = Intimport 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 boardContext
StackExchange Code Review Q#132641, answer score: 3
Revisions (0)
No revisions yet.