Vigenere cipher in Haskell

Question

I implemented the Vigenere cipher in haskell as a part of exercises for Haskell programming from first principles.

Though the code isn't all that long, I feel it could use some refactoring to simplify things (I feel the code I wrote is ugly). One caveat: string inputs should all be in uppercase.

The vigenere function should be applied like so:

vignere "ALLY" "MEET AT DAWN"

which will create the output:

"MPPRAEOYWY"

As it stands, here's the code I implemented:

import Data.Char
-- exercise chapter 11
encode :: Char -> Int
encode x = ord x - ord 'A'
decode :: Int -> Char
decode x = chr (x + ord 'A')
shift :: (Int -> Int -> Int) -> Int -> Char -> Char
shift f x ch = decode $ f (encode ch) x `mod` 26
rightShift :: Int -> Char -> Char
rightShift = shift (+)
leftShift :: Int -> Char -> Char
leftShift = shift (-)
encodeString :: String -> [Int]
encodeString str = map encode str
type Secret = String
type PlainText = String
type CipherText = String
vignereString :: Secret -> PlainText -> String
vignereString secret plain = take len $ cycle secret
 where len = length $ concat $ words plain
vignereCode :: Secret -> PlainText -> [Int]
vignereCode secret plain = encodeString $ vignereString secret plain
vignere :: Secret -> PlainText -> CipherText
vignere secret plain =
 zipWith rightShift code plainNoSpace
 where code = vignereCode secret plain
 plainNoSpace = concat $ words plain
unvignere :: Secret -> CipherText -> PlainText
unvignere secret cipher =
 zipWith leftShift code cipherNoSpace
 where code = vignereCode secret cipher
 cipherNoSpace = concat $ words cipher

Answer 1

It's a nice solution, but very long for a relatively simple task.

First, note that the cipher is called "Vigenère", so vignere would be considered a misspelling.

The type definitions...

type Secret = String
type PlainText = String
type CipherText = String

... provide an illusion of type safety, where there actually isn't any. They are all just Strings.

All of the helper functions (encode, decode, shift, rightShift, leftShift, and encodeString) could just be reduced to a single function:

import Data.Char (chr, ord)
shift :: (Int -> Int -> Int) -> Char -> Char -> Char
shift op offset ch = numToChar $ (charToNum ch) `op` (charToNum offset)
 where
 charToNum ch = ord ch - ord 'A'
 numToChar n = chr $ (n `mod` 26) + ord 'A'

Hiding the charToNum and numToChar helpers makes it easier to see what calls what.

With the generalized shift function defined, the enciphering and deciphering routines could each be one-liners:

vigenere :: String -> String -> String
vigenere secret = zipWith (shift (+)) (cycle secret) . concat . words
unvigenere :: String -> String -> String
unvigenere secret = zipWith (shift (-)) (cycle secret) . concat . words

Here, I've written them using point-free style to emphasize the fact that vigenere secret acts as an enciphering filter on the text.

• Gurkenglas: shift ought not to take offset and ch in the wrong order just because (-) and zip argument orders are inconvenient. I'd instead replace (-) with subtract.
• Chakravarthy Raghunandan: ah I guess I shouldn't have split functionality to so many little functions which actually ended up making things complex. Thanks for the tips :)
• 200_success: @Gurkenglas Actually, placing offset before ch was a deliberate design decision to facilitate currying. You can write a Caesar cipher decipherer as unCaesar = map $ shift (-) 'B'.
• Gurkenglas: Right, and if the arguments were the right way round you could still do unCaesar = map $ shift subtract 'B'. The offset still goes first, but it is passed to the first argument of the operation, not the second.
• 200_success: @Gurkenglas But then 'B' would be the ch, rather than the offset.