First, here's an overview on style and idiom without changing anything too significantly.
import Data.Char
import Data.List
import Data.Maybe
Switching to the current names for these modules. The non-hierarchical names exist only for compatibility.
type Board = [Integer] -- number of objects in each heap
type Heap = Integer -- Heap id
type Turn = (Integer, Integer) -- heap and number of objects to remove
There's really no reason to use Int
except for number-crunching with small integers. Not that it's likely to matter here, but getting in the habit of using Integer
by default means you don't have to worry about bugs due to, say, a counter in a long-running program exceeding the maximum size of Int
. Mystery bugs that only occur on large data sets or after running for a long time are not fun to track down.
The only problem is that some standard functions take only Int
arguments for no good reason, rather than any integral type. This is a misfeature of the standard libraries and not one you should emulate.
applyTurn :: Turn -> Board -> Board
applyTurn (heapId, removed) board = zipWith decHeap [1..] board
where decHeap idx n | idx == heapId = n - removed
| otherwise = n
Several things here:
- Pattern matching on the turn instead of using
fst
and snd
, as well as better names for everything.
- Zipping lists and then mapping a function over that is what
zipWith
is for.
- Replacing the lambda with a function in the
where
clause, and using guards on that rather than an if
expression.
I've left the algorithm unchanged for now. It could be improved, as Paul Martel shows, but using lists for this purpose at all is really not ideal. I'll return to this point later.
availableHeaps :: Board -> [Heap]
availableHeaps b = [heapId | (heapId, count) <- indexedHeaps b, count > 0]
A slightly different way of writing the same list comprehension Paul Martel used. Many Haskell programmers prefer using map, filter, &c. directly; doing so, it would look like this instead:
availableHeaps b = map fst . filter ((> 0) . snd) $ indexedHeaps b
But I think the list comprehension is clearer in this case.
availableObjectsByHeap :: Board -> Heap -> Integer
availableObjectsByHeap board heapId = board !! (fromInteger heapId - 1)
The (!!)
function gives zero-based indexing into a list, so we adjust to account for heap numbers starting from 1. It takes an Int
argument, as noted above. Using (!!)
--or any sort of indexing into a list--continues to be less than ideal, and a sign that some other data structure should be used.
I'll be correcting the spelling of "prompt" as I go, incidentally.
Now, we could try the following to tidy up promptInt
:
promptInt :: String -> (Integer -> Bool) -> IO Integer
promptInt msg p = do
putStr (msg ++ "> ")
x <- readLn -- Don't actually do this!
if p x
then return x
else promptInt msg p
Unfortunately, this is a distinct disimprovement. Using readLn
raises an exception when it can't parse the user input, which makes it needlessly awkward to use. Rather than messing with catching exceptions, we'll whip up a replacement using the reads
function, which returns a list of possible parses, and use Maybe
to indicate success vs. failure.
-- Why doesn't this exist in the Prelude?
maybeRead :: (Read a) => String -> Maybe a
maybeRead str = listToMaybe [x | (x, "") <- reads str]
maybeReadLn :: (Read a) => IO (Maybe a)
maybeReadLn = fmap maybeRead getLine
Now, we can fix promptInt
correctly:
promptInt :: String -> (Integer -> Bool) -> IO Integer
promptInt msg p = do
putStr (msg ++ "> ")
mx <- maybeReadLn
case mx of
Just x | p x -> return x
_ -> promptInt msg p
Using the standard Read
instance instead of doing calculations with ord
makes it easier to see what's going on here. The predicate has also been combined with the pattern match, so the default pattern handles both parse failures and invalid inputs.
promptIntFromRange :: String -> (Integer, Integer) -> IO Integer
promptIntFromRange msg (from, to) = promptInt newMsg inRange
where newMsg = concat [msg, "[", show from, ";", show to, "]"]
inRange v = v >= from && v <= to
It's more typical to have where
begin a new line, in order to clearly distinguish a where
clause from a multi-line expression. Using concat
tends to be tidier than repeated (++)
, and again improving a name--for an arbitrary predicate p
makes sense, but here we have a specific predicate, and should indicate such.
promptIntFromSet :: String -> [Integer] -> IO Integer
promptIntFromSet msg s = promptInt (msg ++ show s) (`elem` s)
This can all be done in-line, since the standard library already has a function for your predicate.
putAllStr :: [String] -> IO ()
putAllStr xs = mapM_ putStrLn xs
You'll probably reinvent large sections of the standard library at various points while learning Haskell. Figuring out that you've done so is half the fun.
printBoard :: Board -> IO ()
printBoard board = putAllStr $ showHeaps board
showHeaps :: Board -> [String]
showHeaps board = map showIdxHeap (indexedHeaps board)
showIdxHeap :: (Heap, Integer) -> String
showIdxHeap (heapId, n) = heapIndex ++ objects
where heapIndex = concat ["[", show heapId, "]"]
objects = genericReplicate n '*'
As Paul Martel did, I've separated the string representation of the board from the printing. Note that String
is simply [Char]
, so replicating '*'
suffices. The use of genericReplicate
here is because of using Integer
rather than Int
.
play :: Board -> IO Board
play board | empty board = return []
| otherwise = do printBoard board
t <- readTurn board
play $ applyTurn t board
nim :: IO ()
nim = do play [1, 2, 3, 1]
putStrLn "done"
There's no reason for play
to take an IO Board
, so I've removed the superfluous return
and binding steps. This also allows using guards for the empty
check, removing the conditional expression.
Ok. With that out of the way, time to revisit the earlier remarks about data structures. Lists in Haskell are sequential in nature, so indexing into them or replacing a single element is clumsy and inefficient at best. We'd like something more suitable here, and a good default choice is the Data.Map
module.
import qualified Data.Map as Map
type HeapId = Integer
type Turn = (HeapId, Integer)
type Board = Map.Map HeapId Integer
The module is imported qualified to avoid name clashes with various list functions. I've also renamed Heap
to HeapId
to be more explicit about what it represents.
applyTurn :: Turn -> Board -> Board
applyTurn (heapId, removed) board = Map.adjust (subtract removed) heapId board
empty :: Board -> Bool
empty b = Map.null $ availableHeaps b
availableHeaps :: Board -> Board
availableHeaps b = Map.filter (> 0) b
Converting the game state functions to use Data.Map
. They're much simpler this way, and some functions I've eliminated entirely.
nim :: IO ()
nim = do play $ Map.fromList (zip [1..] [1, 2, 3, 1])
putStrLn "done"
Initializing the game doesn't need to change much, but note the construction using Map.fromList
, assigning specific keys to each heap by counting from 1.
promptHeapSize :: String -> Board -> IO (HeapId, Integer)
promptHeapSize msg board = do
heapId <- promptInt msg' (`Map.member` board)
case Map.lookup heapId board of
Nothing -> promptHeapSize msg board
Just sz -> return (heapId, sz)
where msg' = msg ++ show (Map.keys board)
Here I've replaced promptIntFromSet
with a smarter function that makes sure the requested heap number is valid, and returns the number of objects as well.
readTurn :: Board -> IO Turn
readTurn b = do
(heapId, heapSz) <- promptHeapSize "heap" b
objects <- promptIntFromRange "number" (1, heapSz)
return (heapId, objects)
showHeaps :: Board -> [String]
showHeaps board = map showIdxHeap (Map.assocs board)
Only a couple very minor changes here.
runNextTurn :: Board -> IO Board
runNextTurn b = do
printBoard board
t <- readTurn board
play $ applyTurn t board
play :: Board -> IO Board
play board | empty board = return board
| otherwise = runNextTurn board >>= play
I split play
into two functions--one that does a single turn, and one that loops until the game is done. This doesn't really change anything, but makes it easier if you want to have more complicated interaction than the same loop every time.
The complete program using Data.Map
, along with some other minor changes I made along the way, can be found here.