Generating a tabula recta

I want an experienced Haskeller to critique my code and offer changes/suggestions (so I can learn the idioms of the language and culture around Haskell).

I've been using a Tabula Recta for my passwords and used a Python script someone wrote to generate the table and do table traversal.

I've been wanting to learn Haskell and decided to take on building the same program but in Haskell.

I'm new to Haskell but not functional programming (I have Scheme and Erlang under my belt - Erlang I use in production on a daily basis). I feel like I kept compositional style but I doubt I adhered to idiomatic Haskell.

{-# LANGUAGE BangPatterns #-}
{- |
Module      :  tabula.hs
Description :  Generates a tabula recta
Copyright   :  (c) Parnell Springmeyer
License     :  BSD3

Maintainer  :  ixmatus@gmail.com
Stability   :  experimental

Generates and manipulates a tabula recta.
-}

module Main where

import Control.Monad.CryptoRandom
import Crypto.Random

import Data.String.Utils
import Data.List.Split

main :: IO()
main = do
    values  IO [[Char]]
tabulay cmap = tabulay' cmap []
    where tabulay' [] acc    = do return acc
          tabulay' (x:xs) acc = do
            chars  IO [Char]
tabulax cint = tabulax' cint []
    where tabulax' 0 acc    = do return acc
          tabulax' iint acc = do 
            val  IO Int
randVal len = do
    g  IO Char
randomChoice ls = do
    randomIndex <- randVal (length ls)
    let val = ls !! randomIndex
    return val

Yet another problem is the usage of lists. !! is O(N) operation. Use Data.Array instead.

As for randoms, you should call newGenIO only once and use the cryptographically secure generator it returned after that instead of creation of new generators each time you need a random character.

Your randVal function should look something like this:

randVal :: Int -> SystemRandom -> (Int, SystemRandom)
randVal len g = fromRight $ crandomR (0, len-1) g

It accepts old generator state and returns the new state to use in subsequent generations. To simplify passing the generator state around you can use a state monad. But there is already such monad - the CRand monad and corresponding CRandT monad transformer to combine random number generation with other monadic computations.

The example from the documentation for CRand fails to typecheck because of monomorphism restriction - getRandPair looks like a value but it isn't, so the compiler detects a potential problem. You can disable it for now by adding {-# LANGUAGE NoMonomorphismRestriction #-} before module Main where line.

Also, it is not clear how the SystemRandom generator works. It is secure, but I think it's not adviced to drain system entropy source. I'd rather use it just to seed HashDRBG from Crypto.Random.DRBG, which is SHA512-based RNG.

Here is a hint:

type Gen a = CRand HashDRBG GenError a

randValM :: Int -> Gen Int
randValM len = getCRandomR (0, len-1)

randomChoice :: [a] -> Gen a
randomChoice ls = do
    randomIndex <- randValM (length ls)
    return $ ls !! randomIndex

Your tabulax function will remain the same, but will operate in a different monad, so it's type will change to randomChoice :: [a] -> Gen a. Note also that I used getCRandomR from Control.Monad.Crypto.Random to make use of the monadic helper to implicitly pass the cryptogenerator around and perform error checking.

Your main will change too:

printGenerated values = do
    putStrLn $ "    " ++ join " " (chunksOf 1 ['A'..'Z'])
    putStrLn $ "  + " ++ (take 52 $ cycle "- ")
    putStrLn $ join "\n" (reverse values)

fromRight (Right a) = a

generate = evalCRand $ do
    a <- tabulay ['A'..'Z']
    return a

main = do
    g <- newGenIO :: IO HashDRBG
    printGenerated $ fromRight $ generate g

The idea is that we extracted generate - a pure function which accepts a generator and returns generated data. evalCRand is used to extract the inner a value from monadic Gen a value. It accepts two arguments: a monadic computation in Gen monad and initial state of random generator. There is also runCRand function which returns the final state of the generator in addition to the value, but we don't need the final state here.

I abused the do-notation here to help you understand code better. I would write generate and main shorter:

generate = evalCRand $ tabulay ['A'..'Z']

main = newGenIO >>= printGenerated . fromRight . generate

Your tabulatex' function is already there in the standard library. It is Control.Monad.replicateM:

import Control.Monad hiding (join)

tabulax :: Int -> Gen [Char]
tabulax cint = replicateM cint $ randomChoice characterMap

tabulatey' is also there in Control.Monad, it's mapM:

tabulay :: [Char] -> Gen [[Char]]
tabulay = mapM $ \x -> do 
            chars <- tabulax 26
            let formatted = [x, '|'] ++ chars
                spaced = join " " (chunksOf 1 formatted)
            return spaced

Next step is to use immutable arrays:

import Data.Array.IArray

tabulax :: Int -> Gen [Char]
tabulax cint = replicateM cint $ randomChoiceA arrMap

arrMap :: Array Int Char
arrMap = listArray (0, (length characterMap) - 1) characterMap

randomChoiceA ls = do
    randomIndex <- randValM $ 1 + snd (bounds ls)
    return $ ls ! randomIndex

Now randValM seems to be perverted: we convert bounds from tuple to size and back. Eliminating the extra conversion we get rid of randValM completely, as getCRandomR and bounds perfectly fit each other:

randomChoiceA ls = do
    randomIndex <- getCRandomR $ bounds ls
    return $ ls ! randomIndex

Now we can shorten it by desugaring do-notation and using function composition and infix operator sections:

randomChoiceA ls = getCRandomR (bounds ls) >>= return . (ls !)

And even more using ` from Control.Applicative (which is the same as fmap):

randomChoiceA ls = (ls !)  getCRandomR (bounds ls)

Finally, we can extract separate = join " " . chunksOf 1 as it is used 2 times and shorten tabulay just for fun:

tabulay :: [Char] -> Gen [[Char]]
tabulay = mapM $ \x -> separate  ([x, '|'] ++)  tabulax 26

and get rid of ugly -1 in array bounds:

arrMap :: Array Int Char
arrMap = listArray (1, length characterMap) characterMap

So the final source is:

``
{-# LANGUAGE NoMonomorphismRestriction #-}
module Main where

import Control.Monad.CryptoRandom
import Crypto.Random.DRBG
imp

StackExchange Code Review Q#15291, answer score: 6