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

Implement total savings function on a record type

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

Problem

the TotalSavings() function on the record below has a code smell that I can't exactly detail. Is there a more readable alternative for implementing this function on a record type?

type ProvidersCharged = ProvidersCharged of decimal
type InsuranceSavings = InsuranceSavings of decimal

type ClaimsSummary = 
    { Claims:Claim seq
      ProvidersCharged:ProvidersCharged
      InsuranceSavings:InsuranceSavings 
    } member this.TotalSavings() = match this.ProvidersCharged with 
                                   | ProvidersCharged charged -> 
                                        match this.InsuranceSavings with
                                        | InsuranceSavings savings -> charged - savings

Solution

Yes, there are, indeed, a couple of code smells.

The first one is that TotalSavings is a type member, and not a function ;)

The other code smell is that you have the beginning of arrow code in your implementation.

I'd write it like this instead:

let totalSavings summary =
    let (ProvidersCharged charged) = summary.ProvidersCharged
    let (InsuranceSavings savings) = summary.InsuranceSavings
    charged - savings


Notice that you don't have to use the match keyword when doing pattern match. With single-case discriminated unions, you can also pattern match directly into let-bound values.

Normally, the brackets aren't necessary, but in this case they are, because e.g. ProvidersCharged is both a case constructor and a record label. If the case constructor had been unambiguous, you could have omitted the brackets, like this:

let Foo foo = bar


You can also pattern match directly in function arguments, but in this case, it becomes too verbose:

let totalSavings' { ProvidersCharged = ProvidersCharged charged; InsuranceSavings = InsuranceSavings savings; Claims = _ } =
    charged - savings


Notice how the record labels are automatically destructured here, using nested pattern matching: matching is done first on the record labels, then on the case constructors inside of the labels.

You'll also notice that I used a wildcard match on Claims because I don't care about that value.

Code Snippets

let totalSavings summary =
    let (ProvidersCharged charged) = summary.ProvidersCharged
    let (InsuranceSavings savings) = summary.InsuranceSavings
    charged - savings
let Foo foo = bar
let totalSavings' { ProvidersCharged = ProvidersCharged charged; InsuranceSavings = InsuranceSavings savings; Claims = _ } =
    charged - savings

Context

StackExchange Code Review Q#142016, answer score: 3

Revisions (0)

No revisions yet.