Refactor Money with TDD - 03

Refactor Money with TDD - 03

GET TO THE PRIMITIVE

While we very much want to avoid data flowing through our system, sometimes we need them. Sometimes we have to get back to a primitive type.

This happens at the boundaries of our system. When our code passes to some other code, we probably have to turn it into a primitive. Two places we're (almost) guaranted to have to transform is sending data across the wire and saving to a database. When we invoke 3rd party code we'll usually have to as well.

This is about how I do that.

Already Are

There's already a couple examples of doing this in the example. The Currency#AsString method and the EqualMoney#explicit operator bool . These are both different ways to transform the object to a primitive type.

Money has it as well for both a primitive, double, and a custom type, Currency. We'll see how we can change this type, if we should. I'm not sure we should; but it's using very similar code, and similar things should be made simiilar. If they aren't actually similar... if they're different then they should be made more different.

Let's hit the boolean first, as that's the most obvious culprit... to me. And I hate booleans. They're primitives I can't get rid of! That's not entirely true... I CAN. It just... would be confusing.

ToSystemType

I've gone through quite a few itterations of how to transform an object into a primitive and I ended up doing a lot of copy/paste between the classes. When that's happening, we're probably missing an abstraction. This is the result of that

[DebuggerDisplay("{AsSystemType()}")]
internal abstract class ToSystemType<TSystemType>
{
    public static implicit operator TSystemType(ToSystemType<TSystemType> origin) => origin.AsSystemType();

    public abstract TSystemType AsSystemType();
}

Really simple. I have an async version as well, but we don't need to bother with that here. I normally put this type of really base abstraction into a lib folder. It changes overtime, I don't overthink it. The file structure changes a lot as we evolve out code.

The story of the evolution for this can be found on here.

We can apply this to our EqualMoneyclass as a base class.

internal sealed class EqualMoney: ToSystemType<bool>

and fix the red that shows up by adding the following method

public override bool AsSystemType() => Value();

ToSystemType already has an implicit operator based on the generic, which for this class is a bool... I bet we can just delete the operator bool in EqualMoney.

And everything stays green, so clearly that works.

Looking at Money#Equals we still have the explicit cast

private bool Equals(Money other) => (bool)new EqualMoney(this, other);

And if your tools are up to snuff, they'll identity the (bool) is no longer needed. We have the implicit operator. The compiler will handle the cast for us.

Oooo.... I think ReSharper adds a little icon indicating an implicit cast is happening.

The EqualMoney#Value method was private and is only used by the EqualMoney#AsSystemType so we can inline that. Our money equality class now looks like this

internal sealed class EqualMoney: ToSystemType<bool>
{
    private const double Tolerance = .0001;
    private readonly Money _lhs;
    private readonly Money _rhs;

    public EqualMoney(Money lhs, Money rhs)
    {
        _lhs = lhs;
        _rhs = rhs;
    }

    public override bool AsSystemType()
    {
        double lhsAmount = (double)_lhs;
        Currency lhsCurrency = (Currency)_lhs;
        double convertedAmount = (double)_rhs.To(lhsCurrency);

        return Math.Abs(lhsAmount - convertedAmount) < Tolerance;
    }
}

The AsSystemType could have some stuff inlined

public override bool AsSystemType()
{
    return Math.Abs((double)_lhs - (double)_rhs.To((Currency)_lhs)) < Tolerance;
}

Which isn't horrible. Once it's established as functional it should never change unless we calculate equality differently. The, essentially, debug variables aren't needed.

To The Primitive

And now our class auto converts to a boolean. I don't like having the methods return booleans. It's a primitive. As a system evolves, I'll change the return type to an abstract class that implements ToSystemType<bool>. It has to be an abstract class as interfaces can't do the protected method hidden-ness.

It'd be a Marker Class, which is the same concept as a Marker Interface except using a class.

public abstract class MoneyAreEqual : ToSystemType<bool>{}

And the MoneyAreEqual type could be the return type. This would ensure that if the answer needed to be passed around and not immediately used, there'd be no question that the right type of answer was being provided.

true has lost ALL meta data. What's true? It's on the developer to understand the system and mentally keep track that "this particular boolean value means 'the money are equal'"... and that's an increase of the cognitive load the developer must maintain while working in the system. If we, instead, provide an object for the concept of "are the money equal" then it's EASY to see that the question being answered is for "money are equal".

This ties directly into Representing the Concept that exists in the code. "Are Money Objects Equal" is a concept in the code; and now, we can have an explicit object for that.

Let's make the change. Even though we're in a small and contrived example; it's good to have the good practices in place.

///EqualMoney.cs
...
internal sealed class EqualMoney: MoneyAreEqual
...
  
//Money.cs
public abstract class Money
...
private MoneyAreEqual Equals(Money other) => new EqualMoney(this, other);
...

And BAM - We're using objects!

Of course the Equals override still returns a boolean; but that's an issue I don't think I'll get to in this.

Exchange Rate?

We could update our exchange rate to return an object that is the ExchangeRate which implements ToSystemType<double>. It wouldn't actually have any public methods. It just allows us to provide an exchange rate as an object. When it's needed to spit out the value, it'll be able to implicitly cast, or explicitly, depending on how well the compiler can resolve what's going on.

Let's do that. It'll be a "hacky" way, but demonstrates the actuality.

First, let's blow out all the interfaces and classes in our ExchangeRates.cs after we rename ExchangeRates to ExchangeRateCollection

With that done, we want to look at the InternalTo which is implementing the IExchangeRateTo interace. We want to create an object that takes in the double, and will return it via the ToSystemType implicit operator.

public abstract class ExchangeRate : ToSystemType<double>{

We'll start with the marker class we know we'll want, then implement the simple example

public sealed class ExchangeRateOf :ExchangeRate
{
    private readonly double _value;

    public ExchangeRateOf(double value) => _value = value;

    public override double AsSystemType() => _value;
}

I have a tendency for the objects that take and return the vale to be name [TYPE]Of. That's just a convention I fell into and haven't found a better alternative.

We can use this object in our InternalTo class to be instantiated and returned. It'll be immediately cast to a double and that's fine; as it'll prove it all works.

public double To(Currency currency)
{
    if (!_map.ContainsKey(currency)) throw new NoExchangeRateAvailableException();
    return new ExchangeRateOf(_map[currency]);
}

With all tests staying green... I think we can call it good.

Next is to update the interface to return the marker class, ExchangeRate. With a minor update to a test to update the Func<double> to Func<ExchangeRate> - We're all green.

Our ConvertToMoney#AmountValue works with no modifcation. This might be a little surprising, so let's take a look at it.

protected override double AmountValue() => (double)_origin * _exchangeRateCollection.From((Currency)_origin).To(_toCurrency);

We're casting the Money _origin into a double allowing the compiler to be smart about the right hand side and cast it for us, implicitly.

ToString

We have one place that's got a cheap version of turning into a primitive, Currency. Let's have it derive from ToSystemType<string> and the required method can just return _currency.  If we just delete the AsString method; we'll get some tests that no longer compile. Currency' does not contain a definition for 'AsString'. What I really like about this aproach, aside from my former colleague loving it - We just have to delete the method from the tests.

string actual = subject.AsString();

to

string actual = subject;

and we should be perfectly back to green.

Yes - We could have went and modified those tests one by one before we removed Currency#AsString. But... Sometimes it's fun to delete the code first.

Our tests are indeed green once again.

[DebuggerDisplay("{_currency}")]
public abstract class Currency : ToSystemType<string>
{
    private readonly string _currency;

    protected Currency(string currency) => _currency = currency;
    public override string AsSystemType() => _currency;
}

We can also remove the ToString method.  Doesn't seem to be getting used anywhere. No reason to leave cruft in.

Show Comments