4
\$\begingroup\$

I've come up with this simple implementation of the Caesar cipher. It takes an integer argument and a file to produce the cipher text like so:

./caesar 4 < text.raw

Here's the code:

import System.Environment
import Data.List
import Data.Maybe

alnum = ['A' .. 'Z'] ++ ['a' .. 'z'] ++ ['0' .. '9']
anlen = length alnum

rotChar :: Int -> Char -> Char
rotChar r c
    | elem c alnum = alnum !! (mod (r + fromJust (elemIndex c alnum)) anlen)
    | otherwise = c

rotStr :: Int -> [Char] -> [Char]
rotStr 0 s = s
rotStr r s = (map (rotChar (mod (anlen + r) anlen)) s)

main :: IO ()
main = do
    args <- getArgs
    contents <- getContents
    putStrLn $ rotStr (read $ head args) contents

I'm new to Haskell and I'd like some opinions on the:

  • global "variables"
  • use of parentheses
  • argument parsing

Could any of this be improved? Any other tips will be appreciated.

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Global "variables"

I would not worry about global variables. Assuming that this code will be properly placed inside its own module, those would probably not be global anyway.

Use of parentheses

Some parentheses are superfluous. For example, the whole body of the second rotStr equation is enclosed in parentheses and those can be removed. Regarding the other cases, you can probably avoid parens (and get slightly easier to digest code) if you define subexpressions in a let or where clause.

Argument parsing

I'm not sure what you mean by that. If you mean the getArgs part, that looks OK.


Here are some other notes about the code:

  • The rotStr 0 s = s line is superfluous. You can remove it and everything will work just fine. I assume it is just an optimization, so that's OK.
  • It might not be obvious for small inputs, but the performance of the code can be improved. Things like elemIndex c alnum and alnum !! run in linear time in the size of alnum. This means that the expression elem c alnum = alnum !! (mod (r + fromJust (elemIndex c alnum)) anlen) will go through the list 2 times. Again, this might not matter much, but you could use Data.Maps for lookup in order to get better performance.

Here's a possible implementation using Maps:

import Data.List
import Data.Maybe
import qualified Data.Map as M (Map, empty, insert, lookup)

alnum = ['A' .. 'Z'] ++ ['a' .. 'z'] ++ ['0' .. '9']
anlen = length alnum

indices = foldr add M.empty $ zip alnum [0..]
  where add (c, i) m = M.insert c i m

charsByIndex = foldr add M.empty $ zip alnum [0..]
  where add (c, i) m = M.insert i c m

rotChar :: Int -> Char -> Char
rotChar r c = case M.lookup c indices of
                   Just i  -> fromJust $ M.lookup (newIndex i) charsByIndex
                   Nothing -> c
              where newIndex i = (i+r) `mod` anlen

rotStr :: Int -> String -> String
rotStr r s = map (rotChar r) s

Also note that the last line can be reduced to:

rotStr = map . rotChar
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Also (!!) is linear time. \$\endgroup\$
    – Code-Guru
    Mar 16, 2018 at 14:44
  • \$\begingroup\$ @Code-Guru RIght, thanks! I've edited the answer to add that too. \$\endgroup\$ Mar 16, 2018 at 14:49
  • \$\begingroup\$ About the argument parsing, I didn't find any "standard" way like Python's argparse. That's why I not entirely sure about the way I did it. \$\endgroup\$
    – qcassimiro
    Mar 19, 2018 at 11:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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