HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Basic random password generator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
randomgeneratorbasicpassword

Problem

I'm trying to pick up some Haskell skills so thought I'd write a random password generator to get to grips with using IO. It was far trickier than I expected and I employed rather more trial and error than I'd have liked.

What I've ended up with works, and while I'm happy with it generally I'd like to know if this is reasonable, idiomatic Haskell or if I've done anything peculiar.

import System.Random

-- Generate a random number in closed interval [lo, hi]
random_int :: Int -> Int -> IO Int
random_int lo hi = (randomRIO (lo,hi) :: IO Int)

-- Generate a random character from an internal alphabet
random_char :: IO Char
random_char = do
    index .?/'"

-- Generate a random password of given length
random_password :: Int -> IO String
random_password length =
    sequence $ map (\x -> random_char) [1..length]

main :: IO ()
main = random_password 8 >>= putStrLn


The implementation of random_password feels odd and hacky, but I struggled to find another way to create an array of results from successive invocations of random_char or any way to avoid the need for sequence. Is it hacky?

Solution

hlint

You should run hlint on your program - it's a good way to become
familiar with techniques and practices used by expert Haskellers.

Running hlint on your code returned the following suggestions:

* use camelCase instead of underscores

./passwd.hs:7:20: Warning: Redundant bracket
Found:
  (randomRIO (lo, hi) :: IO Int)
Why not:
  randomRIO (lo, hi) :: IO Int

./passwd.hs:19:5: Error: Use mapM
Found:
  sequence $ map (\ x -> random_char) [1 .. length]
Why not:
  mapM (\ x -> random_char) [1 .. length]

./passwd.hs:19:21: Warning: Use const
Found:
  \ x -> random_char
Why not:
  const random_char


sequence . map

You're right that the code for random_passwd feels hacky since
the value x is ignored. The canonical way to write this is:

import Control.Monad (replicateM)

random_passwd = replicateM length random_char


replicateM repeats a monadic action a specified number of times collecting the
results in a list.

!!

You probably already know that using !! to index into a list
is inefficient. A better data structure to use for alphabet is
Text from Data.Text which gives you O(1) indexing.

import qualified Data.Text as T

alphabet = T.pack $ ['A'..'Z'] ++ ['a'..'z'] ++ ...

random_char = do
  index <- randomR (0, T.length alphabet - 1)
  return $ T.index alpha index

Code Snippets

* use camelCase instead of underscores

./passwd.hs:7:20: Warning: Redundant bracket
Found:
  (randomRIO (lo, hi) :: IO Int)
Why not:
  randomRIO (lo, hi) :: IO Int

./passwd.hs:19:5: Error: Use mapM
Found:
  sequence $ map (\ x -> random_char) [1 .. length]
Why not:
  mapM (\ x -> random_char) [1 .. length]

./passwd.hs:19:21: Warning: Use const
Found:
  \ x -> random_char
Why not:
  const random_char
import Control.Monad (replicateM)

random_passwd = replicateM length random_char
import qualified Data.Text as T

alphabet = T.pack $ ['A'..'Z'] ++ ['a'..'z'] ++ ...

random_char = do
  index <- randomR (0, T.length alphabet - 1)
  return $ T.index alpha index

Context

StackExchange Code Review Q#106443, answer score: 6

Revisions (0)

No revisions yet.