Last active
January 26, 2017 18:26
-
-
Save beyond-code-github/8711794c4d516cb6941d47274884b248 to your computer and use it in GitHub Desktop.
My take on what good, clean C# looks like in the context of http://functionalsoftware.net/fsharp-rewrite-of-a-fully-refactored-csharp-clean-code-example-612/
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
using System; | |
using System.Collections.Generic; | |
namespace OnlineShop | |
{ | |
delegate decimal Discount(decimal amount, int yearsAccountHeld); | |
public static class Discounts | |
{ | |
public static decimal NotRegistered(decimal amount, int yearsAccountHeld) => amount; | |
public static decimal SimpleCustomer(decimal amount, int yearsAccountHeld) => | |
(amount * 0.9m) * LoyaltyDiscount(yearsAccountHeld); | |
public static decimal ValuableCustomer(decimal amount, int yearsAccountHeld) => | |
(amount * 0.7m) * LoyaltyDiscount(yearsAccountHeld); | |
public static decimal MostValuableCustomer(decimal amount, int yearsAccountHeld) => | |
(amount * 0.5m) * LoyaltyDiscount(yearsAccountHeld); | |
private static decimal LoyaltyDiscount(int yearsAccountHeld) => 1 - (Math.Min(yearsAccountHeld, 5) / 100); | |
} | |
public class DiscountCalculator | |
{ | |
private Dictionary<int, Discount> applyDiscountFor = new Dictionary<int, Discount>() | |
{ | |
{ 0, Discounts.NotRegistered }, | |
{ 1, Discounts.SimpleCustomer }, | |
{ 2, Discounts.ValuableCustomer }, | |
{ 3, Discounts.MostValuableCustomer } | |
}; | |
public decimal Calculate(decimal amount, int accountType, int yearsAccountHeld) => | |
this.applyDiscountFor[accountType](amount, yearsAccountHeld); | |
} | |
} |
Nice rewrite!
You can add a sprinkling of type inference to the rule declarations by declaring them as Discount properties e.g. static Discount SimpleCustomer = (amount, yearsAccountHeld) => (amount * 0.9m) * LoyaltyDiscount(yearsAccountHeld);
The F# (non idiomatic) translation:
type YearsAccountHeld = int
type DiscountAmount = decimal
type Discount = DiscountAmount -> YearsAccountHeld -> decimal
module Discounts =
let inline loyaltyDiscount yearsAccountHeld = 1m - ((min (decimal yearsAccountHeld) 5m) / 100m)
let inline makeDiscount discount (amount: DiscountAmount) (yearsAccountHeld: YearsAccountHeld) =
(amount * discount) * (loyaltyDiscount yearsAccountHeld)
let NotRegistered amount _ = amount
let ValuableCustomer = makeDiscount 0.7m
let MostValuableCustomer = makeDiscount 0.5m
let SimpleCustomer = makeDiscount 0.9m
module DiscountCalculator =
let applyDiscountFor =
dict [
(0, Discounts.NotRegistered)
(1, Discounts.SimpleCustomer)
(2, Discounts.ValuableCustomer)
(3, Discounts.MostValuableCustomer)
]
let Calculate amount accountType yearsAccountHeld = applyDiscountFor.[accountType] amount yearsAccountHeld
// try it right away in FSI
DiscountCalculator.Calculate 100m 0 100 // 100m
DiscountCalculator.Calculate 100m 1 100 // 85.5m
DiscountCalculator.Calculate 100m 2 100 // 66.5m
DiscountCalculator.Calculate 100m 3 100 // 47.5m
Not a vast improvement on the C# but remove most syntactical / keyword cluter
The sum types solution that started this discussion is of course better
For those wondering what's up here
https://www.codeproject.com/articles/1083348/csharp-bad-practices-learn-how-to-make-a-good-code
http://functionalsoftware.net/fsharp-rewrite-of-a-fully-refactored-csharp-clean-code-example-612/
and now this gist and https://twitter.com/beyond_code/status/824367640298844162
😆
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Weirdest part about this code for me (besides
int accountType
) is thatNotRegistered
has an unusedyearsAccountHeld
parameter just to make it fit the other method signatures.You can imagine this approach becoming much worse as the problem domain increases in complexity.