CIS194: Risk!-style "battles"

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)

-- att

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.

StackExchange Code Review Q#118309, answer score: 4