HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Improving F# conditional assignment with match expression

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
expressionwithmatchconditionalassignmentimproving

Problem

In my code I declared a dictionary to store various counters

let counters = Dictionary()


Later, I need to generate several labels using these counters, like this:

  • First type a label is generated for a given type, it should consist of the type name itself:



"MyType"

  • Subsequent label should be the type name, appended with the number of times a label was previously generated for that type, in parentheses:



"MyType (2)"

Here's how I'm accomplishing this now:

let t : Type = ...

let label = 
    if (counters.ContainsKey(t)) then
        counters.[t] <- counter.[t] + 1
        sprintf "%s (%i)" t.Name counters.[t]
    else
        counters.Add(t, 1)
        t.Name


This is exactly how I would write it if I had to use C# (with the exception of the in-line if / else), but after I wrote it F#, it seems clumsy and a little messy. I suspect there is a much more elegant, idiomatic way of writing this in F#. Perhaps using a match expression?

Solution

Thanks to the input provided in the other answers, here's what I've come up with:

let label = 
    let (found, value) = this.counters.TryGetValue(t)
    this.counters.[t] <- value + 1
    if found 
    then (sprintf "%s (%i)" t.Name (value + 1)) 
    else t.Name


Or using a match expression

let label = 
    let (found, value) = this.counters.TryGetValue(t)
    this.counters.[t]  (sprintf "%s (%i)" t.Name (value + 1)) 
    | false -> t.Name


I've also refactored this into a separate function, so the actual assignment is just

let label = getNextLabel(t)


Both of these look better than my original code, though I'm not sure that the match has added any expressiveness to the code, so for now I'm keeping the plain old if / else statement. This is mostly just taking advantage of the built-in behavior of dictionaries in .NET (TryGetValue returns the default value if the key is not found, and the Item property's setter will perform an insert if necessary), but I'm still open to any suggestions on improving this more.

Code Snippets

let label = 
    let (found, value) = this.counters.TryGetValue(t)
    this.counters.[t] <- value + 1
    if found 
    then (sprintf "%s (%i)" t.Name (value + 1)) 
    else t.Name
let label = 
    let (found, value) = this.counters.TryGetValue(t)
    this.counters.[t] <- value + 1
    match found with 
    | true -> (sprintf "%s (%i)" t.Name (value + 1)) 
    | false -> t.Name
let label = getNextLabel(t)

Context

StackExchange Code Review Q#37323, answer score: 3

Revisions (0)

No revisions yet.