F# simple monoalphabetic cipher code implementation

Question

I am a new F# programmer, and a student in a security class. As part of my notes, I wrote a small implementation of a monoalphabetic cipher as follows:

open System
let random = Random()
let defaultCharMap() = 
 ['a' .. 'z'] @ ['A' .. 'Z'] @ ['0' .. '9']
let randomCharMap() = 
 let map = defaultCharMap()
 map
 |> List.sortBy (fun t -> random.Next(map.Length))
 |> List.zip map
 |> Map.ofList
let encode msg = 
 let map = randomCharMap()
 msg
 |> String.map (fun t -> map.TryFind t |> defaultArg <| t)
encode "Hello, world!!!"

The code is designed to take any alphanumeric input, which is then encoded to a random map (done in randomCharMap). This map is simply the plaintext values as keys to a random ciphertext value.

As this is a functional language, I have done my best to use piping and HOF to achieve this. I am looking to validate my work and see if this can be optimised in any way.

• Maarten Bodewes: Undoubtedly not for production use, but Random is not a cryptographically secure RNG.
• TimeTravelPenguin: Yeah, no issues there. Just random code I threw together to play with F# and my university material :) Later this semester we implement our own security systems and I am well prepared for proper implementation ;) @MaartenBodewes

Answer 1

Your code looks good in terms of being idiomatic functional-ish F#. 👍🏾

I do still have some suggestions:

• defaultCharMap is a function that always returns the same value. Therefore it might as well be a plain module-level value instead. This will mean it's evaluated when the module is loaded (essentially just before the program starts). However, if you only want it to evaluate it once just before it is first needed you can make it a lazy value, and then request the value using the .Value property. Also, it is not a map so I would call it chars.

• When building up chars you're using @ to append lists. This can be slow when the list on the left is quite long. This is probably not an issue at all given the lists are small but it might be better to prefer a list comprehension.

• 4 space formatting is much more common than 2 spaces in F# code.

• The t in the List.sortBy function is not used so the convention is to discard the value by naming it _.

• People generally avoid using defaultArg, preferring Option.defaultValue instead. The latter has a better parameter order that allows easy piping without the need for the backwards pipe <|. It's usually recommended to avoid using <| as the operator precedence can be confusing.

With all of those suggestions applied, the code would look like this:

open System
let random = Random()
let chars = lazy [
 for c in 'a' .. 'z' do c
 for c in 'A' .. 'Z' do c
 for c in '0' .. '9' do c
]
let randomCharMap() = 
 chars.Value
 |> List.sortBy (fun _ -> random.Next(chars.Value.Length))
 |> List.zip chars.Value
 |> Map.ofList
let encode msg = 
 let map = randomCharMap()
 msg
 |> String.map (fun t -> map.TryFind t |> Option.defaultValue t)
encode "Hello, world!!!"

You could arguably make the code more functional by passing in the Random as an argument to any function that needs it, instead of accessing a static Random. This would mean that the functions could be considered to be more pure and you could pass in a seeded Random which always produces the same result, allowing predictable testing.

• TimeTravelPenguin: Awesome!! I never considered list comprehension in that way, that is a great tip! I am a C# dev, and use 2 spaces since it fits more on screen, but I will keep that in mind for when sharing code! I didn't consider passing random as an arg, but that makes a lot of sense. Thank you very much for the criticism! I very much appreciate it in order to become better at FP :)
• TimeTravelPenguin: One question I forgot to ask: rather than a list for chars, would a Seq be better? iirc isn't it lazy? Or is it the elements that are lazy?
• TheQuickBrownFox: @TimeTravelPenguin Seq is just another name for IEnumerable. So yes it is lazy but it will be evaluated again every time it is used. The F# lazy is guaranteed to only evaluate once. But the trade off in this case is that the list will be held in memory for the duration of the program even if it is no longer used.