Friday, September 17, 2010

Implementing a ‘Money’ type in an ASP.NET MVC and NHibernate application.

Years ago I read Kent Beck’s seminal Test Driven Development. The first third of the book is a complete worked example of building a Currency type using TDD and I remember thinking at the time that this was an incredibly powerful technique. Rather than using basic data types -  int, string, DateTime - we could define what we actually meant. Rather than having a property Age of type int, we would define an Age type. Rather than two string properties we could define a Name type. One of the factors in moving Suteki Shop from Linq-to-SQL to NHibernate was NHibernate’s support for more complex finer-grained mapping.

When I first hacked together Suteki Shop, I used the decimal type everywhere I needed to represent money. This has mostly worked well for the simple scenario where a shop only has a single currency and that currency is sterling (£). But anyone wanting to sell in US dollars had to find every one of the many instances of “£” and replace them with “$”, and if you wanted to do something fancy with multiple currencies, you would have been out of luck. What I needed was a Money type. It’s not a trivial refactoring. Here are the steps I needed to take:

  1. Create a Money type that behaved as far as possible like a .NET numeric type.
  2. Create an ASP.NET MVC IModelBinder that knew how to bind the Money type.
  3. Create an NHibernate IUserType that knew how to persist the Money type.
  4. Change every point in the code that was using decimal to represent money to use the Money type instead.

Create a Money type

I wanted to create a Money type that would behave nicely when I did arithmetic with it. So I wanted to be able to write expression like this:

var x = new Money(43.5M);
var result = x * 3 + x/2 - 4;
result.Amount.ShouldEqual(148.25M);

To achieve this you have to write a lot of operator overloads for all combinations of operator, Money <-> Money, Money <-> decimal and decimal <-> Money. I won’t bore you, if you want to see the gory details, you can view the code here.

I also wanted my Money type to include the currency symbol when ToString was invoked, so I would get “£12.54” rather than “12.54”, but I found that this caused lots of problems with HTML forms and the default behaviour of the MVC HtmlHelper extensions. In the end I left ToString returning the number without the currency symbol and implemented a new method ToStringWithSymbol with it included. It feels like a horrible hack though.

I also overrode the equality operator, GetHashCode and Equals so make Money behave as expected in equality operations.

public class Money
{
    public decimal Amount { get; private set; }

    public Money(decimal amount)
    {
        Amount = amount;
    }

    public static string Symbol
    {
        get { return "£"; }
    }

    public static Money Zero
    {
        get { return new Money(0M);}
    }

    public override string ToString()
    {
        return Amount.ToString("0.00");
    }

    public string ToStringWithSymbol()
    {
        return Amount.ToString(Symbol + "0.00");
    }

    public override int GetHashCode()
    {
        return Amount.GetHashCode();
    }

    public override bool Equals(object obj)
    {
        var otherMoney = obj as Money;
        if (otherMoney == null) return false;
        return Amount.Equals(otherMoney.Amount);
    }

    public bool Equals(Money otherMoney)
    {
        if (otherMoney == null) return false;
        return Amount.Equals(otherMoney.Amount);
    }

    public static bool operator ==(Money a, Money b)
    {
        // If both are null, or both are same instance, return true.
        if (ReferenceEquals(a, b))
        {
            return true;
        }

        // If one is null, but not both, return false.
        if (((object)a == null) || ((object)b == null))
        {
            return false;
        }

        return a.Amount == b.Amount;
    }

    public static bool operator !=(Money a, Money b)
    {
        return !(a == b);
    }
    
    .....
}

Create an ASP.NET MVC IModelBinder

I wanted any of my forms that posted money values to work seamlessly when binding to entities with Money properties. For this to work I had to implement an IModelBinder for Money.

Creating the ModelBinder was pretty straightforward since I was just binding to a simple type.

public class MoneyBinder : IModelBinder
{
    public object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
    {
        if (!typeof (Money).IsAssignableFrom(bindingContext.ModelType))
        {
            throw new SutekiCommonException(
                "MoneyBinder has attempted to bind to type '{0}', but may only bind to Money",
                                            bindingContext.ModelType);
        }

        var valueProviderResult = bindingContext.ValueProvider.GetValue(bindingContext.ModelName);

        try
        {
            var decimalValue = (decimal)valueProviderResult.ConvertTo(typeof(decimal));
            return new Money(decimalValue);
        }
        catch (Exception exception)
        {
            bindingContext.ModelState.AddModelError(bindingContext.ModelName, "Not a valid price.");
            return null;
        }
    }
}

But it could have been much easier. The IModelBinder interface doesn’t make it easy to work out how you are supposed to do simple binding. A SimpleModelBinder<T> with an abstract method like: T BindSimpleModel(string value) would have been nice.

The MoneyBinder is registered as a binder for the Money type in Global.asax.cs:

ModelBinders.Binders.Add(typeof(Money), new MoneyBinder());

Create an NHibernate IUserType

I wanted any entity that had a property of type Money to get automatically persisted by NHibernate as a numeric value. To do this I needed an IUserType. This tells NHibernate how to persist your custom type.

To create the NHibernate IUserType I borrowed Darrell Mozingo’s excellent BaseImmutableUserType<T>. With this it’s fairly straightforward:

public class MoneyUserType : BaseImmutableUserType<Money>
{
    public override object NullSafeGet(IDataReader rs, string[] names, object owner)
    {
        var amount = ((decimal?)NHibernateUtil.Decimal.NullSafeGet(rs, names[0]));
        return amount.HasValue ? new Money(amount.Value) : Money.Zero;
    }

    public override void NullSafeSet(IDbCommand cmd, object value, int index)
    {
        var moneyObject = value as Money;
        object valueToSet;

        if (moneyObject != null)
        {
            valueToSet = moneyObject.Amount;
        }
        else
        {
            valueToSet = DBNull.Value;
        }

        NHibernateUtil.Decimal.NullSafeSet(cmd, valueToSet, index);
    }

    public override SqlType[] SqlTypes
    {
        get
        {
            return new[]
                   {
                       SqlTypeFactory.Decimal
                   };
        }
    }
}

To register the MoneyUserType I just added a convention to my Fluent NHibernate setup:

public class FluentNHibernateConfigurationBuilder : IConfigurationBuilder
{
    ....

    public static void ConfigureMappings(MappingConfiguration mappingConfiguration)
    {
        mappingConfiguration.FluentMappings
            .AddFromAssembly(typeof (ProductMap).Assembly)
            .Conventions.Add(
                ForeignKey.EndsWith("Id"),
                PrimaryKey.Name.Is(x => x.EntityType.Name + "Id"),
                DefaultCascade.None(),
                new MoneyConvention());
    }
}

public class MoneyConvention : UserTypeConvention<MoneyUserType>{}

Change every point in the code that was using decimal to represent money to use the Money type instead

I had naively hoped that a good enough Money class would allow me to just change the type of money properties from decimal to Money and have done with it. But even with the operator overloading I found that there were many places where I needed to add ‘.Amount’ to make the code behave. I could have implemented an implicit cast, but I don’t like not knowing where types are being converted. I’ve found in the past that it can lead to subtle, hard to find, bugs.

What I have now is a single point where I can change the way money behaves in Suteki Shop. Providing multi-currency or changing the behaviour of money in any way should be far easier. And if you simply want to take Suteki Shop and sell in Dollars, Yen or Euros, it’s now just a single line change to the code. And yes, configurable currencies will come very soon.

Some thoughts on the C# type system

This was far more work than it needs to be and is a clear example of the deficiencies of the C# type system. With Haskell’s type classes or Scalar’s implicits it would have been possible to create a new numeric type without having the change ripple through the entire application. The Scalar implicit is a very interesting idea and would surely be a very cool addition to the C# type system. Anders?

7 comments:

Chris said...

You could simply leverage the current culture for ToString() support.


return Amount.ToString("c");

Mike Hadlow said...

Thanks Chris, that's a very good call, I didn't know that "c" as a format output the currency symbol.

However, in my case it's probably not sufficient, since I'm building a multi-tenanted application where different tenants will want to use different currencies. Excellent suggestion though.

Richard OD said...

I was going to suggest what Chris had said, but other than that, why did you create this as a class and not a struct?

It looks like a perfect candidate for a struct to me.

I'm sure I've read about this kind of approach in one of Fowler's books- there is an interesting article here, if you are interested: http://martinfowler.com/ap2/quantity.html

Richard OD said...

Anyone read this may also find the following interesting:

http://stackoverflow.com/questions/2215601/money-data-type-for-net

Richard OD said...

Sorry Mike for the multiple comments, but if you had this a struct, you could then leverage Nullable:

Nullable someNullableMoney = new Money(20);

someNullableMoney = null;

Etc.

Huseyin Tufekcilerli said...

Another reason to consider using built-in .NET currency formatting is that not all currencies print symbols on the left of the amount. i.e.:

var s = 42M.ToString("C", CultureInfo.GetCultureInfo("tr-TR")); // "42,00 TL"

Michel said...

You have to know that Money(342.11M).ToString() will return "342,11" on a french server. This break ToString_should_not_include_currency_symbol() and ToStringWithSymbol_should_include_currency_symbol() tests on SutekiShop.

And worse, it's break Orders_ShouldReturnACsvFileOfOrders() which expects a 11 items array and gets a 13 items array.

I think that numbers in CSV files should always use the "." as decimal separator.