Improving F# conditional assignment with match expression

In my code I declared a dictionary to store various counters

let counters = Dictionary<Type, int>()


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
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?

-

Another improvement would be to close over counters, assuming it's only used by this function.

let mkLabel =
let counters = Dictionary()
fun (ty: Type) ->
match counters.TryGetValue(ty) with
| true, curCount ->
let newCount = curCount + 1
counters.[ty] <- newCount
sprintf "%s (%i)" ty.Name newCount
| _ ->
ty.Name

//Usage
mkLabel typeof<int>
mkLabel typeof<string>

-
Good suggestion. Yes, counters is only used for the purpose of generating labels in this function. – p.s.w.g Dec 13 '13 at 22:40

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] <- value + 1
match found with
| true -> (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.

-
Wrapping parentheses for sprintf are not needed. – Dmitry G. Nov 13 '15 at 4:30

You can indeed use match, together with the fact that F# converts methods with out parameters (like Dictionary.TryGetValue()) to methods returning tuples:

let label =
match counters.TryGetValue(t) with
| (true, counter) ->
counters.[t] <- counter + 1
sprintf "%s (%i)" t.Name counter
| (false, _) ->
t.Name


Though I'm not sure whether this is actually any better.

Few more notes:

• I think that t should be a parameter of label.
• Using a mutable collection like Dictionary in F# doesn't feel right. I guess the “right” way to do it in functional style would be something like:

label (map : Map<Type, int>) (type : Type) : (Map<Type, int>, string)


But this would lead to code that's even less clear.

-
Thanks for your input. Please see my answer below. Hopefully the rewrite is clear an idiomatic. – p.s.w.g Dec 13 '13 at 22:22