CIS194: Risk!-style "battles"

Question

After reading LYAH and using RWH as a reference, I've been doing the exercises from CIS194, which is often suggested to beginners on irc://freenode.net/#haskell and by this author. I've just completed Homework 12, which goes with Lesson 12 and focuses on Monads, using the template provided here.

You can find the complete exercise requirements here.

It apparently works, but I would love some suggestions, stylistically and otherwise, to turn it into proper, idiomatic, clean, readable, good Haskell. I'm especially concerned by how monad-specific >>, return and >>== and are under-represented.

{-# LANGUAGE GeneralizedNewtypeDeriving #-}

module Risk where

import Control.Monad.Random
import Control.Applicative
import Data.List
import Data.Functor.Identity

------------------------------------------------------------
-- Die values

newtype DieValue = DV { unDV :: Int } 
  deriving (Eq, Ord, Show, Num)

first :: (a -> b) -> (a, c) -> (b, c)
first f (a, c) = (f a, c)

instance Random DieValue where
  random           = first DV . randomR (1,6)
  randomR (low,hi) = first DV . randomR (max 1 (unDV low), min 6 (unDV hi))

die :: Rand StdGen DieValue
die = getRandom

------------------------------------------------------------
-- MY CODE STARTS HERE
------------------------------------------------------------
-- Risk

-- Exercise 2

-- Given the definitions

type Army = Int

data Battlefield = Battlefield { attackers :: Army, defenders :: Army }

-- (which are also included in Risk.hs), write a function with the type

-- battle :: Battlefield -> Rand StdGen Battlefield

-- which simulates a single battle (as explained above) between two
-- opposing armies. That is, it should simulate randomly rolling the
-- appropriate number of dice, interpreting the results, and updating the
-- two armies to reflect casualties. You may assume that each layer will
-- attack or defend with the maximum number of units they are allowed.

roll n = sequence (replicate n die)

-- attacker can attack with x <= 3 armies, but must leave one in place
maxattackers :: Battlefield -> Int
maxattackers b = maximum [0, minimum[3, ((attackers b) - 1)]]

-- defender can defend with x <= 2 armies
maxdefenders :: Battlefield -> Int
maxdefenders b = maximum [0, minimum[2, (defenders b)]]

-- build [(a1,d1), (a2, d2), ...] pairs of dice rolls sorted and matched according to Risk! rules
fight attackers defenders = zip
                            <$> descending (roll attackers)
                            <*> descending (roll defenders) where descending =  fmap(reverse . sort) 

-- from list of rolls infer number of armies lost by each side
losses :: [(DieValue, DieValue)] -> (Int, Int)
losses ds =  (alosses ds,
              dlosses ds)
  where awins (x,y) = (x > y); -- Attacker wins when attacker's dice is strictly > 
        alosses = (length . filter (==False) . fmap awins); -- Attacker's losses is when attacker does not win
        dlosses = (length . filter (==True)  . fmap awins); -- Viceversa

battle :: Battlefield -> Rand StdGen Battlefield
battle b =
  let attacking = maxattackers b;
      defending = maxdefenders b in
      (updatefield . losses) <$> (fight attacking defending)
      where updatefield (alost,dlost) = (Battlefield (attackers b - alost) (defenders b - dlost))



-- Exercise 3

-- Of course, usually an attacker does not stop after just a single
-- battle, but attacks repeatedly in an attempt to destroy the entire
-- defending army (and thus take over its territory).

-- Now implement a function

-- invade :: Battlefield -> Rand StdGen Battlefield

-- which simulates an entire invasion attempt, that is, repeated calls to
-- battle until there are no defenders remaining, or fewer than two
-- attackers.

invade :: Battlefield -> Rand StdGen Battlefield
invade b = case b of
  Battlefield 1 _ -> return b
  Battlefield _ 0 -> return b
  otherwise -> battle b >>= invade

-- Exercise 4

-- Finally, implement a function

-- successProb :: Battlefield -> Rand StdGen Double

-- which runs invade 1000 times, and uses the results to compute a Double
-- between 0 and 1 representing the estimated probability that the
-- attacking army will completely destroy the defending army.

-- For example, if the defending army is destroyed in 300 of the 1000
-- simulations (but the attacking army is reduced to 1 unit in the other
-- 700), successProb should return 0.3.


-- take list [a] and return percentage of list elements satisfying given condition
percentageFilter :: (a -> Bool) -> [a] -> Double
percentageFilter cond xs = (fromIntegral.length.(filter cond)) xs  /   (fromIntegral.length) xs

repeatInvasion :: Int -> Battlefield -> RandT StdGen Identity [Battlefield]
repeatInvasion n b = (sequence(replicate n (invade (b)))) 

successProb :: Battlefield -> Rand StdGen Double
successProb b = (percentageFilter attackerWins <$> (repeatInvasion 1000 b)) where attackerWins = ((==0).defenders)

Answer 1

Your code looks good to me: I have only minor suggestions:

Use weaker functions when possible

maximum works for a list of any length, while max for just two items, the same for min

maxattackers b = max 0 (min 3 ((attackers b) - 1))

The lack of square braces helps readability a bit.

Small descriptively named functions

In the above you are basically constraining (attackers b) - 1 to be in (0, 3) range. I suggest writing a function for it, as you do that in maxdefenders too:

constrainToRange x (a, b) = max a (min b x)

And you get an improvement in readability and deduplicate:

maxattackers b = constrainToRange ((attackers b) - 1)) (0, 3)
maxdefenders b = constrainToRange (defenders b) (0, 2)

Use filter directly

When you say:

filter (==True) . fmap awins

It is equivalent to:

mfilter awins

That is simpler.

Longer names and less repetition

The losses function has non-descriptive names (awins?) and repetition:

        alosses = (length . filter (==False) . fmap awins); -- Attacker's losses is when attacker does not win
        dlosses = (length . filter (==True)  . fmap awins); -- Viceversa

Using mfilter as suggested above and taking advantage that the attacker xor the defender loses:

losses ds =  (attackerLosses, defensorLosses)
  where
    attackerWins (x,y) = (x > y)
    defensorLosses = length $ mfilter attackerWins ds
    attackerLosses = length ds - defensorLosses

Such names also make the comment redundant.