No Primitives

I'm VERY against primitives' being passed around in a system. As soon as I started putting in hard coded strings of the currency I knew it'd have to be extracted out into something that's not a primitive.
I didn't do this earlier because it wasn't being asked for by the code. We kept it contained within our system, so it wasn't as concerning. Now, with our need to remove currency specific methods, callers are required to provide the currency, we can't have them typing the hard coded string. It's much to easy to make a mistake like that.

One of my favorite ways to determine the complexity of a system is how much data moves around. The more data movement in the system the more complex it is. That complexity creates friction to maintainability. I want my systems to be easy to maintain, so I try to prevent complexity as best as I can.

Let's get onto our current requirement

5 USD * 2 = 10 USD
10 EUR * 2 = 20 EUR
4002 KRW / 4 = 1000.5 KRW
5 USD + 10 USD = 15 USD
5 USD + 10 EUR = 17 USD
12 USD == 10 EUR
1 USD + 1100 KRW = 2200 KRW
2.50 USD * 6 = 15 USD
Convert between currencies
Currency not exposed primitives
Remove named currency methods for general one

We can do an enum with the currency as the string name. We could just use the int value of the enum, it's only our system, so should be OK.
I've actually been thinking about how a consumer could extend the system to provide their own currency types and their own exchange rates... which... Yeah... I don't think int values would be very useful. Especially with how our ExchangeRatesdata is set up; that'd be terrible.

Let this be an important bit about how design decisions impact the code evolution. While we can very much change everything of our data struct, especially since we've kept it nice and isolation - What it currently is does constrain our choices a bit, unless we want to put in the extra work.

This is one of the reasons I don't want to extract things too early, It constrains future decisions unless we do the work to undo/change the abstraction. If we have to undo the abstraction, it was clearly not the right abstraction.
We won't always make the right abstraction. It's almost certain it'll have to change, but too early means more changes. It's a balance, you'll learn your comfort level with time.

OK - If an enum is out. What's next... public const string? That doesn't really prevent strings from floating around our system. We would still have string as our param types, which is what we want to avoid.

The only one really left is a type that take a string, and can produce it on demand.

public class Currency{
    private readonly string currency;
    protected Currency(string currency) => _currency = currency
}

As the basic. We still need a way to read the value out. And create the values.

Let's see what this looks like implemented.

public class Currency
{
    private readonly string _currency;
    protected Currency(string currency) => _currency = currency;
    public string AsString() => _currency;
}

Basically identical. Now; I've quickly coded this up without any tests. I wanted to see what it'd look like and how it'd feel to me.
There's some bits I'm not thrilled with, but what I like to do will require pulling in some techniques outside the scope of the TDD exercise we're going through. You'll see them implemented after we get through all the requirements.

For now, I'm good with it; but let's TDD this into existence.

And yes, I'm doing the exists test.

[TestMethod]
public void Exists()
{
    new Currency();
}

Which won't compile until we use our tool to generate the code

public class Currency
{
}

OH YEAH!

And now let's create the constructor to take a string. To simplify the series of changes, I will use the Exists test to change the constructor.

[TestMethod]
public void Exists()
{
    new Currency("USD");
}

I alternate between this and a new test depending on how I'm feeling.
Which gives us

public class Currency
{
    public Currency(string currency)
    {

    }
}

Aside from naming the variable, I haven't written any of that code. If I'd been planning ahead, I'd have used a currency string in the test of "currency" as the tool uses the value as the initial name. Then I'd have to write zero code of the class. Use the tools.

The next simple test is for the AsString method. One of the reason I like to sketch out a class before I start the test is to see how it looks. Is it the shape I want? Does it have the functionality I'm thinking? Is there anything obvious I missed?
While pairing, it's a quick way to get on the same page before applying TDD to generate it. I'll do it during mobbing as well... but definitely not as often. And almost never during strict mobbing. It tries to force everyone on the same page, but I always feel it limits the navigator choice of what to do during their time. Just go when mobbing, the mob will course correct. The dynamic in pairing is different and the early sketches can help align understanding and enable better progress.

OK; the test.

[TestMethod]
public void AsStringExists()
{ 
    string actual = new Currency("blort").AsString();
}

While forcing the method to exist doesn't require a return type, this provides more information to the tool allowing it to generate the code I want so I don't have to type it.

public string AsString()
{
    return "";
}

And the only thing I had to type here was return "" to prevent it from throwing an exception. Which TECHNICALLY would still have the method existing... but it'd fail unless I added exception expectations. Extra code, not doing it.

The next test we want is to ensure that the returned value is the value passed into the constructor. But we can triangulate to get there.

[TestMethod]
public void AsStringReturnsUSD()
{
    //Arrange
    Currency subject = new Currency("USD");

    //Act
    string actual = subject.AsString();

    //Assert
    actual.Should().Be("USD");
}

A quick and easy test.

nd... HAHAHA - I made a mistake that's very hard to show (because I'm lazy and not doing screen shots right now). My test runner didn't auto-include the CurrencyTests so while I was working with non-compiling tests I didn't notice that my Green state wasn't running the tests. It wasn't until I had a test that would fail on an assert that I got confused. I was all green... but... that assert would not pass.

Oops.

Make sure your tests are running. That's key.

Now that it's running; I can see that it fails for the reason we expect. Expected actual to be "USD" with a length of 3, but ""

Let's make it pass!

public string AsString()
{
    return "USD";
}

That's right! Hard Code the Value!!!

Since we want the exact same test with a different value; let's follow our

• Copy & Paste
• Edit Values
• Edit Name

process to create a new test. And this fails for the reason we expect Expected actual to be "Anything" with a length of 8, but "USD" has a length of 3, differs near "USD" (index 0). Looking at our existing implementation... kinda hard to do anything but the right thing.

public class Currency
{
    private readonly string _currency;

    public Currency(string currency) => _currency = currency;

	public string AsString() => _currency;
}

Which gets us pretty close. We should refactor the class out now.

Object => Values?

We have the object, now how do we construct the values?

We could do the knowledge classes as we did before. That's not terrible. It's allow us to make Currency abstract. I haven't made it sealed at the moment since I'm not sure what I want it to be.

I don't really like the Knowledge Classes for this. I don't really want many instance of objects floating around. A currency should be reference equal to the same currency.
That means they have to be static. Like this

public class Currency
{
    public static Currency UsDollar = new("USD");
    public static Currency KoreanWon = new("KRW");
    public static Currency Euro = new("EUR");
	...
}

I don't like that it's not abstract or sealed. A huge concern I have if I seal it is that currencies can't be extended or added by consumers. We're limiting our system by doing that. I could abstract it, but then I'd have to create an internal derived class just to instantiate it. Which feels like overkill.

This is one of the few times an object will stay neither sealed nor abstract. Unless... Depending on the system, I might make it abstract with the private derived class to keep the "always sealed or abstract" pattern in the code base. It's much harder to allow an exception when there's no exceptions.

And I've talked myself into it.

OK, first we need tests to create the new currencies.

[TestMethod]
public void UsdAsCurrency()
{
    //Arrange
    Currency subject = Currency.UsDollar;

    //Act
    string actual = subject.AsString();

    //Assert
    actual.Should().Be("USD");
}

The tests for each one follows this pattern. It quickly allows us to implement the required static values.

Now that we have these, we can create our internal derived class to allow us to abstract Currency.

AKSHULLY... As mentioned, not a fan of the internal private. I'm gonna switch it to be abstract but have the derived class be external to Currency. We'll have to shift some other stuff around.

I've quickly hacked in

public abstract class Currency
{
    private readonly string _currency;

    protected Currency(string currency) => _currency = currency;

    public string AsString() => _currency;
}

public sealed class DefaultCurrency: Currency
{
    public static Currency UsDollar = new DefaultCurrency("USD");
    public static Currency KoreanWon = new DefaultCurrency("KRW");
    public static Currency Euro = new DefaultCurrency("EUR");
    private DefaultCurrency(string currency) : base(currency){}
}

Which breaks a lot of tests.

I'm gonna just update the tests. I feel this is a refactor; but a RISKY refactor. This is definitely a R!! for the commit notation.

The test updates actually reduces it down to 3 tests, one for each currency type

[TestMethod]
public void UsdAsCurrency()
{
    //Arrange
    Currency subject = DefaultCurrency.UsDollar;

    //Act
    string actual = subject.AsString();

    //Assert
    actual.Should().Be("USD");
}

This confirms all of the behaviors we want.

There sure is a lot of duplication in these tests like we had before. Does it cry out for simplification? Not to me.

If it does cry out for simplification; try. Listen to the code. If you're hearing "SIMPLIFY ME!!!" try to. Practice is the only way you'll be able to hear the code more accurately.

It is repetition; but it's doesn't cry out duplication to me. The tests all flow differently. There's no way to reduce the code so the tests all hit a single path. The lack of obvious simplification in the code, it's not something the code is calling out for.

Update the places

Now that we have the currency in place; we should update the places using currency to use these new things.

5 USD * 2 = 10 USD
10 EUR * 2 = 20 EUR
4002 KRW / 4 = 1000.5 KRW
5 USD + 10 USD = 15 USD
5 USD + 10 EUR = 17 USD
12 USD == 10 EUR
1 USD + 1100 KRW = 2200 KRW
2.50 USD * 6 = 15 USD
Convert between currencies
Currency not exposed primitives
Remove named currency methods for general one

The biggest impact is the ExchangeRates; so let's go there. We need to support both string and Currency types to be able to migrate the rest of the system and maintain a green state.

This is where I like to go step by step green, and a great colleague prefers to make the change and chase the compilation issues until it works again. I think either is OK. When I pair with him I absolutely use his method. It helps me see other approaches. For this, Let's go the "always green" approach.

What we need to fix first is our ExchangeRates#From method. Well... not really fix, but create an overload and take a Currency. public IExchangeRateTo From(Currency currency).
How do we handle the body though? Until the Dictionary changes... we can't use currency directly. So let's use AsString and call the string method.

public IExchangeRateTo From(Currency currency) => From(currency.AsString());

Should we write a test for this?

We're green. We're refactoring. Do we write tests while refactoring? No. The process here will update the existing behaviors currently under test. Once we're done, our tests will be updated and working against the new method. No new tests will be required to have the same behavior coverage.

What to update now?

With this method, where do we update? Where do we use currency strings? Where do we use the From method? In Money.

Plus ,Equals, and To are all using the From method. Only one of these can switch to the new Currency type; To. That is, via the ToWon and ToDollar methods.

Which... Crap. The currency we have control over is the To Fluent Method... Well then... Lets go do that one first.

We'll overload the To method to have a Currency version which just uses the string version.

public double To(Currency currency)
{
    return To(currency.AsString());
}

Our Money#To method and it's callers now look like

public Money ToWon() => To(DefaultCurrency.KoreanWon);

public Money ToDollar() => To(DefaultCurrency.UsDollar);

private Money To(Currency currency)
{
    double exchangeRate = _exchangeRates.From(_currency).To(currency);
    return new(_amount * exchangeRate, currency.AsString());
}

We can now update the To tests in ExchangeRatesTests to use the Currency overload.

The next thing we need to approach is where else uses the string version of the IExchangeRateTo#To method? It looks like it's still in Money. The Plus and Equals methods. A quick and simple way to find these is make the method scope private and find the compile errors.

Both of these methods use the this._currency type into the To Method... which means we need to construct Money with a Currency. It's times like this that I do think fondly of the "break it" approach.

The "stay green" way is a little slower, but it forces you to understand what's being used where and gives you confidence you're not breaking something along the way. I like that confidence and improved or confirmed understanding of the code.

Since we've created a sealed class and all instances are internal to money; pretty easy/quick update. To stay green, we're going to create a temporary constructor to convert from a Currency to the string.

private Money(double amount, Currency currency):this(amount, currency.AsString()){}

Some simple constructor chaining.

Now we can update the Won, Euro, and Dollar methods to provide the currency.

public static Money Won(double amount) => new(amount, DefaultCurrency.KoreanWon);
public static Money Euro(double amount) => new(amount, DefaultCurrency.Euro);
public static Money Dollar(double amount) => new(amount, DefaultCurrency.UsDollar);

We're still green.

Let's do a little bit of a bigger change; let's change our _currency type to be a Currency. This will be a bit of "break it and fix". While it's possible to do a slow shift... it's over kill. All the breaks will be internal to Money. When it's all in the class you're working in; I'm OK with the "break it" approach.

A couple calls to ExchangeRates#From require the AsString call on the _currency showing we still have that to fix up. Which... if I'd done the interface update when I added the From(Currency) method, I wouldn't need those. So... doing that now.

The question is now, what uses the string versions of the fluent interfaces? Let's delete them from the interfaces and find out!

Some tests in ExchangeRatesTests, which are easily fixed.

Except the one validating the exception thrown from the From method. We need to create a new Currency that's not in our exchange rates.

A quick test only private subclass of Currency sets us up to get back to Green.

That's (almost) all

Except for the dictionary; that's all of the default values... so let's hit the dictionary!

Again, all the breaking happens in this one file, so we can do the sweeping changes required here w/o trying to stay green for every little change inside the class.

I didn't actually have a good distinction for when I want to stay green and when I'm fine with the breaking changes. Contained to the same file, assuming good file practices, is a simple heuristic which seems to fit most of the times I can recall being good with it.

Having switched everything from the hard coded currency strings to the new Currency type, we're still green.

That went a bit faster and smoother than I'd expected.

I should probably go through and add the changes to this. I did make a bunch of small commits. Hopefully I catch this and do that. But not now... at this 2am.

Method Removal

One of the goals from removing the primitive currency types was to be able to remove the ToTYPE methods on Money.

I think we can do that now. All we need to do is make Money#To public and start to use it. Knowing that the ToWon and ToDollar methods are used just in the tests, I'm gonna delete them and use the "break it" approach to this.

public Money To(Currency currency)
{
    double exchangeRate = _exchangeRates.From(_currency).To(currency);
    return new(_amount * exchangeRate, currency);
}

The changes to this are pretty minimal. Just the //Act in the test to use the new To method with the currency

[TestMethod]
public void ConvertFromUsdToWon()
{
    //Arrange
    int value = new Random().Next(1, 20);
    Money subject = Money.Dollar(value);

    //Act
    Money actual = subject.To(DefaultCurrency.KoreanWon);

    //Assert
    actual.Should().Be(Money.Won(value * 1100));
}

We might notice a similarity to the static create methods. Those are type specific. We should get rid of those and have the consumer provide the type. they know it, let them provide it.

With our defined types, it'll work out just fine.

A few changes in for the MoneyTests to instantiate instead of call the static method. Fortunately the inline function of our tools does that for us very nicely. As well as let us update the constructor to public. No typing and even though it's a bit of code change, we can trust it as it's a tool done provable refactor.

Let's Refactor

Let's listen to our tool and the recommendations they give. One thing I missed was our types in DefaultCurrency could be replaced. I didn't have them as readonly. An oversight that our tool caught and helped us correct.

Looking at Money#Equals the guard clause seems ... off. It's doing the same thing as if we let it get the exchange rate.
Let's change from

private bool Equals(Money other)
{
    if (this._currency == other._currency) return _amount.Equals(other._amount);
    double exchangeRate = _exchangeRates.From(other._currency).To(this._currency);
    return _amount.Equals(other._amount * exchangeRate);
}

to

private bool Equals(Money other)
{
    double exchangeRate = _exchangeRates.From(other._currency).To(this._currency);
	return _amount.Equals(other._amount * exchangeRate);
}

And all our tests pass. Since our exchange rate is 1; we get the same result. We don't need to have the extra code - CODE REMOVED! Deleting code is the best feeling.

That's all of our requirements

5 USD * 2 = 10 USD
10 EUR * 2 = 20 EUR
4002 KRW / 4 = 1000.5 KRW
5 USD + 10 USD = 15 USD
5 USD + 10 EUR = 17 USD
12 USD == 10 EUR
1 USD + 1100 KRW = 2200 KRW
2.50 USD * 6 = 15 USD
Convert between currencies
Currency not exposed primitives
Remove named currency methods for general one

Return of the Knowledge

While we're in the refactor phase of our TDD process, I like to follow an approach a fantastic colleague has been using.

We can't just refactor the code we're actively working in. Those are local changes and refactors, which are very important to do. But we need to slowly look at the bigger and bigger picture. Go from the method to the class. From the class to other classes. Look around the code. Look at how things relate. As we go up in scope, we don't have to pay as much attention; but go through your code. Check things out. As your understanding of the system evolves, along with the system, you'll often find things that can be refactored and improved that you didn't see before.

One of my favorite aspects of doing this is directly tied to why I enjoy mob programming - I'm out of the weeds. When you're making the changes, you're in the weeds. You shouldn't be particularly cognizant of the bigger picture when you have to write a line of code.

When you can take the time to look through the code, you're going to be less in the weeds; you'll see the code as it is, not for what it's solving for you. In mob programming, as a Mobber, your role is to be aware of the bigger picture and help the navigator see those things as well.

In this case, as I looked through our latest changes the tests are showing me how the consumers will interact with our code. The tests are complaining to me about creating Money.

The most egregious of these is the test for type equality

[TestMethod]
public void SameTypesAreEqual()
{
    //Arrange
    Random random = new();
    int value = random.Next(1, 20);
    List<Money> source = new() { new((double)value, DefaultCurrency.Euro), new((double)value, DefaultCurrency.KoreanWon), new((double)value, DefaultCurrency.UsDollar) };
    List<Money> compare = new() { new((double)value, DefaultCurrency.Euro), new((double)value, DefaultCurrency.KoreanWon), new((double)value, DefaultCurrency.UsDollar) };
    int index = random.Next(0, source.Count);

    //Act
    bool actual = source[index].Equals(compare[index]);

    //Assert
    actual.Should().BeTrue();
}

The creation of those lists is... huge. What's the issue? Why didn't that stand out before... It wasn't as much code before.

Ignoring the syntax sugar C# is providing, how do we create Money

new Money(value, DefaultCurrency.Euro)

That's a bit of code. Not a lot, but a bit. It requires two pieces of knowledge, Knowing Money and knowing how to access the correct currency.

This double knowledge requirement is what lead to the original creation of Knowledge Classes around creating the different types of money. Now, we're looking at the same duplication, but without the risk of typos due to primitives.

One of the guides I use when developing systems is that anything the users of an API ALWAYS have to do, we do it for them.
In this case they always have to know money and the currency type. I don't like that, clearly. Let's go back to  Currency specific knowledge classes that handle that for the users.

First thing we need to do is un-sealed Money. This will allow us to derive classes from it.

As all Knowledge Classes will be, it's very simple

public class UsDollar:Money
{
    public UsDollar(double amount) : base(amount, DefaultCurrency.UsDollar){}
}

This is almost a convivence object for our consumers. Almost. it ties into my guiding principle of software development - "Represent the concepts that exist in the code".

The above Knowledge Class is a concept that exists in the code - An Amount of US Dollars. When we limit it to creation by new Money(value, DefaultCurrency.UsDollar) then the concept of "An Amount of US Dollars" isn't represented in our code. It exists in our code, clearly - We're creating it right there. We are creating that concept, but it doesn't exist in the code-base itself.

The Knowledge Classes are enabling us to fully represent the domain of our problem with concrete instances that represent the concepts we're using. Once we give these things a name, it becomes much easier to think about them. We've decoupled our thoughts and conversations about UsDollar from always being tied to "an instance of Money". Aside from being the base type, used most places, we have the concept we can talk about.
While this contrived example isn't as obvious, What if the currency code changes? It doesn't happen a lot; but it has, and I'm sure it will again.
The Mexican Peso, in 1993 changed from a currency code of MXP to MXN. I know this because I'm a little bit of a collector of money, and the Peso went from 20,000 Peso bills to 200 Peso bills in the new form. Note: I'm not sure the exact exchange, but that kinda change.

If we allow our consumers to specify the currency code everywhere... they have to now go update ALL of their code to the new currency code. This is instead of just updating a library if we add the concept of a MexicanPeso.
We can also represent the old peso if that's what they needed to use by adding a new type of MexicanPesoPre1993. This is the concept we want to talk about in the code.

new Money(value, DefaultCurrency.MexicanPesoPre1993)

Isn't the concept. That is an instance of Money using the "Pre 1993 Mexican Peso". The conversation about code is centered around "Money". It's a extra mental manipulation we need to have; increasing the cognitive load of thinking about the system. With just MexicanPeso and MexicanPesoPre1993 we have the concepts! We can point to the thing we mean!

Let's add the other two types. These, we could definitely add tests to generate...

[TestMethod]
public void UsDollarExists()
{
    //Arrange
    Money subject = new UsDollar(10);

    //Act
    Money actual = subject.To(DefaultCurrency.UsDollar);

    //Assert
    actual.Should().Be(subject);
}

Simple little things that will convert to the same type, and should continue to be equal.

We're going a little faster with these tests as we've already built out the concepts when we had the hard coded currency. We could be doing these as pure refactor phase, if we wanted. We're adding them to replace the instantiation of Money, so there will be tests for all of the knowledge these classes contain.

Once these classes are in

public class UsDollar : Money
{
    public UsDollar(double amount) : base(amount, DefaultCurrency.UsDollar) { }
}
public class Euro : Money
{
    public Euro(double amount) : base(amount, DefaultCurrency.Euro) { }
}
public class KoreanWon : Money
{
    public KoreanWon(double amount) : base(amount, DefaultCurrency.KoreanWon) { }
}

we can update the rest of the tests to use this instead of Money directly. To simplify this, I'll use my colleagues approach of "break it"!. Let's make the public constructor for Money protected!

That'll give us some errors. Which we fix!

The big method from above is much reduced

[TestMethod]
public void SameTypesAreEqual()
{
    //Arrange
    Random random = new();
    int value = random.Next(1, 20);
    List<Money> source = new() { new Euro(value), new KoreanWon(value), new UsDollar(value) };
    List<Money> compare = new() { new Euro(value), new KoreanWon(value), new UsDollar(value) };
    int index = random.Next(0, source.Count);

    //Act
    bool actual = source[index].Equals(compare[index]);

    //Assert
    actual.Should().BeTrue();
}

Still a bit to the list, but it's far less noisy.

Currently Money is not following our abstract or sealed practice. We clearly can't seal it, so let's make it abstract. Since we do need to be able to create instance of Money without knowing the currency type, we have to have some 'general form' we can create instances of. As mentioned earlier, I'll use private nested or internal classes. This is a case I think the private class is going to be our best option. It's cleanest and minimizes the exposure in our own system for things we don't want others to do.

BUT FIRST - Seal up the knowledge classes.

OK; simple.

What we end up with is a pass through that allows us to instantiate Money

private sealed class InnerMoney:Money
{
    public InnerMoney(double amount, Currency currency) : base(amount, currency){}
}

It doesn't feel the best. Never has, but it's hidden from the rest of the system. It doesn't add any complexity outside of Money and minimal there. It's a little smell... but ... something else I do will remove this nested class.

Well done!

That's for suffering through my writing.

Next up

I promised to show a couple things through this exercise. I don't remember what they are... so hopefully when I edit this I'll find them again. The ones I know I want to share are

• PrivateCtor - Utility class to access private constructors
• ToSystemType - Not mentioned during this, but a utility I use to simplify the AsString behavior
• MicroObjects - We haven't hit the size limit yet; let's explore that.
• ADD THE ONES I'M FORGETTING AT 2 AM
• ...