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
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.
t in the
List.sortBy function is not used so the convention is to discard the value by naming it
People generally avoid using
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:
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() =
|> List.sortBy (fun _ -> random.Next(chars.Value.Length))
|> List.zip chars.Value
let encode msg =
let map = randomCharMap()
|> 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.