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.

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.