patternMinor
Idiomatic conditionals in F#
Viewed 0 times
idiomaticconditionalsstackoverflow
Problem
I'm learning F#, and even though I'm able to do whatever I want, parts of my code looks really bad. I would love to get some suggestions on how to improve a couple of functions involving some conditional logic.
I'm guessing there are ways to make these much more elegant. For instance, the second function uses side-effects on a Dictionary and has return type unit, but I added a branch where I had to use ignore in (probably) a silly way. Please don't change the functionality, but show me a nicer way to write the code.
let stripLast (punct : char) (word : string) =
if word.Length > 0 && word.LastIndexOf(punct) = word.Length - 1
then word.Substring(0, word.Length - 1)
else word
let countWord (dictionary : Dictionary) word =
if word = "" || (blacklisted word) then "" |> ignore
elif dictionary.ContainsKey(word) then dictionary.[word] <- dictionary.[word] + 1
else dictionary.[word] <- 1I'm guessing there are ways to make these much more elegant. For instance, the second function uses side-effects on a Dictionary and has return type unit, but I added a branch where I had to use ignore in (probably) a silly way. Please don't change the functionality, but show me a nicer way to write the code.
Solution
This is probably the way I would write it:
Here's a rundown of the changes:
In
In
A more functional approach, assuming you have a list of words, might be:
let stripLast punct word =
match String.length word with
| n when n > 0 && word.[n-1] = punct -> word.[..n-2]
| _ -> word
let countWord (dictionary : Dictionary) = function
| null | "" -> ()
| word when blacklisted word -> ()
| word ->
match dictionary.TryGetValue(word) with
| true, n -> dictionary.[word] dictionary.Add(word, 1)Here's a rundown of the changes:
In
stripLast:- Use
String.lengthinstead of a type annotation (punctcan also be inferred)
- Capture the length (which is used multiple times) using pattern matching
- Check only the last char (
LastIndexOfsearches the whole string)
- Use string slicing over
Substringfor conciseness
In
countWord:- Allow the type args for
Dictionaryto be inferred
- Use pattern matching to clearly show the cases to be handled
- Replace calls to
ContainsKeyandItem([]) with one call toTryGetValue
A more functional approach, assuming you have a list of words, might be:
words
|> Seq.countBy (stripLast stripChar)
|> Seq.filter (fun (word, _) ->
match word with
| null | "" -> false
| _ -> not (blacklisted word))Code Snippets
let stripLast punct word =
match String.length word with
| n when n > 0 && word.[n-1] = punct -> word.[..n-2]
| _ -> word
let countWord (dictionary : Dictionary<_,_>) = function
| null | "" -> ()
| word when blacklisted word -> ()
| word ->
match dictionary.TryGetValue(word) with
| true, n -> dictionary.[word] <- n + 1
| _ -> dictionary.Add(word, 1)words
|> Seq.countBy (stripLast stripChar)
|> Seq.filter (fun (word, _) ->
match word with
| null | "" -> false
| _ -> not (blacklisted word))Context
StackExchange Code Review Q#24401, answer score: 5
Revisions (0)
No revisions yet.