10
\$\begingroup\$

Learn You a Haskell gives an exercise to implement the following function:

-- make a function that simulates tossing a function 3 times
import System.Random

tossCoin3 :: Int -> (Bool, Bool, Bool)
tossCoin3 x = 
    let (num1, gen1) = random (mkStdGen x)   :: (Int, StdGen)
        (num2, gen2) = random (gen1)         :: (Int, StdGen)
        (num3, _)    = random (gen2)         :: (Int, StdGen)

    in (convertToBool num1, convertToBool num2, convertToBool num3)
    where convertToBool y = if y `mod` 2 == 0 then True else False

I would've mapped a list of [num1, num2, num3] with the convertToBool, and then convert to a 3-tuple. But then I saw this post.

Please critique my implementation.

\$\endgroup\$
8
\$\begingroup\$

Converting a Bool to a Bool with an if

if tests Bool values and returns results. You’re using it to return True if the input is True, and False if the input is False:

convertToBool y = if y `mod` 2 == 0 then True else False

But that’s just the identity function. You can remove the if:

convertToBool y = y `mod` 2 == 0

Useless parentheses

random (gen1)
random (gen2)

Generating random Ints rather than the type you wish to get directly

You’re using random to generate an Int. That’s fine if you want an Int—but you want a Bool. Sure, you can calculate a Bool out of one, but random can also give you a Bool directly. This would simplify your implementation.

I feel it’s necessary to point out the dangers of limiting random numbers to a range with modulo. In this case, it’s not actually a problem, but pretend you want to generate a number between 0 and 6, and the random number generator you have gives you numbers between 0 and 9. If you used modulo, you’d map:

0, 6 --> 0
1, 7 --> 1
2, 8 --> 2
3, 9 --> 3
4    --> 4
5    --> 5
6    --> 6

That’s not a very even distribution. The problem arises because 7 and 10 do not share a factor. It’s not relevant to you because you’re trying to generate something in the range 0 to 1 (two values) and two is a factor of the number of values in Int, but it’s something to be aware of nonetheless.

Dependence on mkStdGen

Right now, you let the user pass in a seed that is then passed to mkStdGen. That works, I suppose, but it would be more flexible if you pushed that responsibility to the user, because then the user can create an StdGen however they desire (with, say, getStdGen). Your signature would then look like this:

coinToss3 :: StdGen -> (Bool, Bool, Bool)

You could even change your definition to allow any RandomGen instance:

coinToss3 :: (RandomGen g) => g -> (Bool, Bool, Bool)

Potential cleanup with MonadRandom and Applicative

At your current point in Learn You A Haskell, you haven’t learned about Monad or Applicative yet, so you may want to ignore this for now and take a look at it later, but using MonadRandom and Applicative, your code can be considerably shortened:

import Control.Applicative
import Control.Monad.Random

coinToss3 :: (RandomGen g) => g -> (Bool, Bool, Bool)
coinToss3 = evalRand $ (,,) <$> random <*> random <*> random
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.