Haskell encryption tips

Question

I wrote a variation of the Caeser Cipher and would like some feedback. Is it idiomatic? Can you generally see anything that can be improved?

Use example:

let e = enc "Hello World" "This is the key"
 in dec e "This is the key"

Instead of taking a single value as the offset, it takes a String as the key and uses each respective character from the key as the offset; wrapping when it reaches the upper/lower bound.

module Encryption where

import Data.Char

type Key = String
type Message = String
type Direction = (Int -> Int -> Int)

--Upper and lower ascii code bounds
upperBound = 127 ; lowerBound = 32

shift :: Char -> Char -> (Int -> Int -> Int) -> Char
shift char charoff dir = fix $ dir (ord char) (ord charoff)

fix :: Int -> Char
fix charcode
        | charcode > upperBound = chr $ (lowerBound - 1) + excessOf (upperBound + lowerBound)
        | charcode < lowerBound = chr $ (upperBound + 1) - excessOf (lowerBound - lowerBound)
        | otherwise = chr charcode
        where   excessOf boundry = abs(boundry - charcode)

alter :: Message -> Key -> (Int -> Int -> Int) -> Message
alter msg key dir = [(shift c k dir) | (c,k) <- zip msg (cycle key)]

enc msg key = alter msg key (+)
dec msg key = alter msg key (-)

--Basic Caeser Cipher
{-
alter :: Message -> Key -> Direction -> Message
alter m k dir = [chr $ dir (ord out) k | out <- m]

enc m k = alter m k (+)
dec m k = alter m k (-)
-}

Answer 1

type Direction = (Int -> Int -> Int)

These parentheses are superfluous, though they don't really hurt if they help you to read the code.

upperBound = 127 ; lowerBound = 32

It's not very common to put definitions on the same line with ;, particularly top-level definitions, so this made me double-take a bit. It makes it hard to find a particular definition by scanning the left column of the file.

It's a good idea to give all top-level definitions type signatures, as it helps to catch type errors earlier and makes it easier for someone reading the code.

Also, with numeric types, overloading in the Num class can often give surprising results, so explicitly stating that these are Int is a good idea.

shift :: Char -> Char -> (Int -> Int -> Int) -> Char
shift char charoff dir = fix $ dir (ord char) (ord charoff)

Consider putting the function argument first here. The function is more likely to be useful partially applied as shift dir than as shift char, and it also makes for a more intuitive type signature which correctly gives the sense that a function Int -> Int -> Int is being transformed into a function Char -> Char -> Char.

fix :: Int -> Char

As someone mentioned on your StackOverflow post, fix is a standard library name for something different, so best not to reuse it here.

| charcode > upperBound = chr $ (lowerBound - 1) + excessOf (upperBound + lowerBound)
| charcode < lowerBound = chr $ (upperBound + 1) - excessOf (lowerBound - lowerBound)
    where   excessOf boundry = abs(boundry - charcode)

I find it hard to understand these two cases. You could perhaps add comments to explain what they are doing. Also lowerBound - lowerBound is just 0 so what are you trying to do here? I presume the whole thing is about clamping to within the valid ASCII range but it's quite confusing.

In general I'd have expected a Caesar cipher just to work on letters, rather than the whole ASCII range, but that's a specification question really.

alter :: Message -> Key -> (Int -> Int -> Int) -> Message
alter msg key dir = [(shift c k dir) | (c,k) <- zip msg (cycle key)]

Again I would put the function as the first argument here. I would also put the Key as the second argument, because then you can partially apply it with a key to get a specialised encryption or decryption function. It might also make sense to have the key first and the function second.

enc msg key = alter msg key (+)
dec msg key = alter msg key (-)

As before, type signatures would help here. Also, if you do change the order of the arguments to alter above then this can be abbreviated as enc = alter (+) or enc key = alter key (+), which is perhaps slightly more elegant.