Enigma machine simulator

As my first (keep that in mind!) Haskall program (full code at end) I've written a simple Enigma machine and would like feedback on the core code related to its stepping and encoding — stripped of comments to see how it fares without them. (I expect to post a follow on where a more complete package is reviewed; but this should stand on its own.)

This code allows for the creation of a machine from a simple specification:

ghci> let cfg = EnigmaConfig ["AE.BF.CM.DQ.HU.JN.LX.PR.SZ.VW","VIII","VI","V","β","c"] [1,25,16,15,25,1] [1,12,5,16,5,1]

which can then be represented in one of the conventional ways:

ghci> windows cfg
"CDTJ"

Examined more deeply:

ghci> putStr $ unlines $ stageMappingList cfg
EFMQABGUINKXCJORDPZTHWVLYS
IXHMSJVNZQEDLURFBTCOGYPKWA
COVLDIWTNEHUARGZFXQJBMPYSK
XJMIYVCARQOWHLNDSUFKGBEPZT
QUNGALXEPKZYRDSOFTVCMBIHWJ
RDOBJNTKVEHMLFCWZAXGYIPSUQ
EVTNHQDXWZJFUCPIAMORBSYGLK
HVGPWSUMDBTNCOKXJIQZRFLAEY
MUAEJQOKFTZDVIBWSNYHLCGRXP
ZQSLKPUCAFXMDHTWJOERNGYBVI
EFMQABGUINKXCJORDPZTHWVLYS

and used to encode messages:

ghci> let msg = "FOLGENDESISTSOFORTBEKANNTZUGEBEN"
ghci> let msg' = enigmaEncoding cfg msg
ghci> msg'
"RBBFPMHPHGCZXTDYGAHGUFXGEWKBLKGJ"
ghci> msg == enigmaEncoding cfg msg'
True

I'm especially interested in review of any errors or missed opportunities to exploit Haskell features or idioms. I'm also curious how clear the code — on its own, with out comments — is.

I'm also particularly interested review of following aspects:

Overall, this is a bit (perhaps considerably?) less efficient that many alternative approaches might be, because rather determining encoding based only on a minimal state specification, I determine the complete mapping of each component of the machine as part of my encoding calculations. Effectively, I determine what the encoding of every letter at every stage would be, and then use that to figure out how a given (single) letter is encoded. This also lets me compute encodings by rotating mappings,

step

This code:

step ec = EnigmaConfig {
        components = components ec,
        positions = steppedPosition  stages ec, 
        rings = rings ec
    }
    ...

may be written:

step ec = ec { positions = steppedPosition  stages ec }

Not only is it shorter, but it is also clearer that the returned value is
the same same as the input ec except that the positions field has changed.

enigmaEncoding

This expression:

(and $ (`elem` letters)  msg)

may be more simply written as:

all (`elem` letters) msg

and is a lot more readable. In general I avoid using ` on lists - just write map. To me, using fmap or ` is a signal that
a more complex structure is involved, like a monad. If it is just a list, it's better to write plainer code - it'll make it a lot easier for someone else to understand.

26

The number 26 appears as a magic constant. It should be given a symbolic name. Or, find a way to structure your code so that you don't need it.

subexpressions

I would try to give good names to sub-expressions in your program.

For instance, in this expression:

zipWith encode (enigmaMapping  (iterate step (step ec))) msg

it's hard to see what the two arguments are. Perhaps write it as:

zipWith encode states msg
  where states = ...

Another example - these expressions are similar:

windowLetter ec st = chrA0 $ mod (positions ec !! st + rings ec !! st - 2) 26
...
    steppedPosition i = (mod (positions ec !! i + di - 1) 26) + 1

I'm sure you can define sub-expressions which make it clearer what's going on.

architecture

This site has a nice picture of how an Enigma machine encodes a letter:

http://enigma.louisedade.co.uk/howitworks.html

Would it be possible to write your code to mimic this diagram? E.g.:

encodeLetter :: EnigmaConfig -> Char -> Char
encodeLetter ec = plug' . right' . middle' . left' . reflect .
                             left . middle . right . static . plug
  where plug = ...
        static = ...
        ...

Someone who is reading your code would be looking for where these parts of the encoding process appear in your code, and structuring the code like this would make it obvious what's going on.

StackExchange Code Review Q#105771, answer score: 8