I'm Not Adding

As mentioned, What about converting between currencies without adding currencies together? That seems like it'll be a useful thing for our system. Let's mark that as our current work in progress!

~~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

While I expect this to be pretty straight forward, I suspect it'll drive some larger changes.

First things first- of course - A TEST!

[TestMethod]
public void ConvertFromUsdToKrw()
{
    //Arrange
    Money subject = Money.Dollar(10);

    //Act
    Money actual = subject.ToKrw();

    //Assert
    actual.Should().Be(Money.Won(11000));
}

I do like this one line AAA style tests. It's so precise.

Our ToKrw method doesn't exist yet. And I don't like it already, but let's get to green and I'll explain why.

Using our wonderful hard coding, we can get our test to pass with ease!

public Money ToKrw()
{
    return Money.Won(11000);
}

Of course this isn't the final form, but I do love the triangulation steps. The practice of doing it for the small stuff helps me keep the habit of doing it for the big stuff. You want to be in the habit of using the good practices when it isn't as important so you don't skip them when they are very important. We can see that the amount of work and refactoring to triangulate is very low. Over time, the amount of code not written by using triangulation to get to only the code you need, with the tests to prove it does what's expected, will save a lot of re-work and prevent bugs. It'll be very clear if you've gotten all of the requirement covered.

Oh Wow! I just realized I haven't used one of my TDD mantras in this exercise yet... I've kinda used it internally, but I haven't been explicit - I'll do that after our next triangulation test.

Copy Paste
Change 11000 to 22000
Change 10 to 20
Change name to ConvertFrom10UsdTo22000Krw

And I've realized I dropped the ball on the thing I dislike... Right... I'm queueing up things to explain. Let's track them for a moment. It's more for me than for you. Time passes much differently writing and coding than it will just reading.

Why I don't like the method
What Mantra?!

OK, there we go. Now; we have our next test, let's get it passing

public Money ToKrw()
{
    if (_amount == 20.0) return Money.Won(22000);
    return Money.Won(11000);
}

A simple update following our practice of hard coding until we see a pattern. I'd like one more test to put the pattern into the code... which... we're not really making the solution show up here. We'll need a different currency to make the pattern show.

Though... since I didn't add a lot of exchange rates; let's just talk about it.

The test I'd write for 20 EUR to KRW. This would force the currency to be included in the if block.

public Money ToKrw()
{
    if (_amount == 20.0 && _currency=="USD") return Money.Won(22000);
    if (_amount == 20.0 && _currency=="EUR") return Money.Won(????);
    return Money.Won(11000);
}

And at this point we'd be able to see that we're just doing conversions of our current value into Won. Which is pretty easy to implement, we have an example of it.

public Money ToKrw()
{
    double exchangeRate = _exchangeRates.From(_currency).To("KRW");
    return Money.Won(_amount * exchangeRate);
    if (_amount == 20.0) return Money.Won(22000);
    return Money.Won(11000);
}

Seeing we're all green, we can then delete the unreachable code.

The Points

The things I want to talk about; First - What don't I like! I don't like ToKrw. I should have made it ToWon. Our creation methods use non-currency terms. I don't know if that'll be the final form, but we should keep similar things similar. We're creating and converting; similar operations which should follow similar practices. If you let everything be "whatever" without consideration of the bigger picture of the class, your code will be a nightmare to work in, and for others to update. It'll be a nightmare for developers to use! If your API isn't consistent, it's not predictable. These prevent developers from building a mental model of how your system should behave and it is a lot more taxing to interact with that you've built. It has to be thought about, or dug into, every time. If a method does what is expected is always suspect. An inconsistent API is always going to be suspect on if it's doing what's expected of it. Let's make things that do similar things follow similar patterns. In this case we want our conversion method to be ToWon which mirrors the pattern established by the Money.Won method.

This is an easy rename refactor: ToKrw -> ToWon

OK, Next Point A Mantra.
This is something I use a lot when coaching teams on how to think about testing.

Can you write another test for the requirement that will fail?

If you can think of a test that will fail, you'll know you haven't fully implemented the requirement. When we started down the through experiment of multiple currencies

public Money ToKrw()
{
    if (_amount == 20.0 && _currency=="USD") return Money.Won(22000);
    if (_amount == 20.0 && _currency=="EUR") return Money.Won(????);
    return Money.Won(11000);
}

We could keep going forever of another value and another currency. This if block triangulation style can go on forever... if you aren't triangulating. Once a pattern is seen, we can start to extract out and generalize the code.

In our current state

public Money ToWon()
{
    double exchangeRate = _exchangeRates.From(_currency).To("KRW");
    return Money.Won(_amount * exchangeRate);
}

Is there another test to convert to Won that we can write that will fail?

Nope. That's... it. Anything that can convert to Won will successfully convert now. We don't need a failing test as that's behavior of the ExchangeRates and we don't need to verify it everywhere it's used. This is one of the reasons I love to extract out small classes. I can re-use the same behavior through the code and never have to test it again.

We're not done converting yet; we still need to do Dollar and Euro. And I'm getting ahead of myself

We're green; we refactor. Our last 2 tests definitely need some work. Both to generalize and to get renamed.
I won't bother renaming both tests to change Krw to Won, but it's a good thing to recognize that we don't want to expose the currency code value outside of where it's required. This is why we changed from ToKrw to ToWon.

Since we've only implemented a conversion between Dollar and Won... that's all we can actually use in out random bit. As well as keeping it tied to the hard coded exchange rate.

Our updated test, generalized, is:

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

    //Act
    Money actual = subject.ToWon();

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

We have the hard coded conversion, but we're putting off forcing a mocked exchange object. Oooo... I have no mocks so far, interesting.

Moving on...

Euro to Dollar

I'm going to follow our testing pattern of copy/paste and update; which gives me the follow

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

    //Act
    Money actual = subject.ToDollar();

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

Good job if you noticed the problem there. Give it a look, see if you notice it. Regardless, I want the test to fail for the reason I expect, so let's dump in a default and see what our exception is

public Money ToDollar()
{
    return Money.Dollar(0);
}

Our failure is Expected actual to be 13.2 USD, but found 0 USD. Perfect; let's update out implementation to return 13.2 USD and we'll get to green.

Or... Not...? Expected actual to be 4.8 USD, but found 13.2 USD.

Our expected changed... As we're going random.

Our copy and paste included our randomization of our input. It's REALLY hard to write hard coded implementations against changing values. Which is why we refactor into the changing value, not start there.
Let's swap to a constant value so we can get to green and start to triangulate

[TestMethod]
public void ConvertFromEuroToDollar()
{
    //Arrange
    int value = 10;
    Money subject = Money.Euro(value);

    //Act
    Money actual = subject.ToDollar();

    //Assert
    actual.Should().Be(Money.Dollar(value * 1.2));
}

We absolutely could shortcut. Again - Practice the small steps on the small things. It'll save you when you get to the big things.

What we want to avoid by doing this is the thought of "How do I solve this?". We don't care. Solving the requirement is /never/ our goal with TDD. It's the outcome of TDD. It's a result of following the cycle, not a goal of the cycle. The cycle has no goal, just steps. Repeat the steps until you cannot do RED.

RED - Simplest test that will fail.
GREEN - Simplest code that makes the test pass.
REFACTOR - Simple changes to generalize patterns in the code.

That's what TDD is to me. Simple tests forcing simple code exposing patterns to generalize. Trying to implement "the solution" is going to bite you. It will. When, I can't say, but it will.

We stop writing tests for a requirement when we can no longer find a simple test that will fail. If we can devise a complex test that will fail; we can break it down until a failing simple test falls out. And if not... Your code is too complex. Refactor that into simpler code.

Back to our tests; we need another test. Let's follow our copy&paste/edit/rename flow to get the following

[TestMethod]
public void ConvertFrom10EuroTo24Dollar()
{
    //Arrange
    int value = 20;
    Money subject = Money.Euro(value);

    //Act
    Money actual = subject.ToDollar();

    //Assert
    actual.Should().Be(Money.Dollar(value * 1.2));
}

and it fails for the reason we expect Expected actual to be 24 USD, but found 12 USD. Our new value is not equal to our hard coded value. Our hardcoded fix is simple.

public Money ToDollar()
{
    if (_amount == 20) return Money.Dollar(24);
    return Money.Dollar(12);
}

And very similar to what we had for ToWon. That, of course, makes sense. Being the sharp developers we are, we understand that further tests will drive the same result, so let's go ahead an follow what we did in ToWon.

public Money ToDollar()
{
    double exchangeRate = _exchangeRates.From(_currency).To("USD");
    return Money.Dollar(_amount * exchangeRate);
    if (_amount == 20) return Money.Dollar(24);
    return Money.Dollar(12);
}

With our Green result we can refactor out the dead code, and our tests to a single general form.

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

    //Act
    Money actual = subject.ToDollar();

    //Assert
    actual.Should().Be(Money.Dollar(value * 1.2));
}

Both the code and tests for the ToCurrency code are VERY similar. This is the code shouting at us. We have duplication in our system.

There's something we can do the simplify the code, it's shouting at us so loud. It's painful to see such similarities in the code and tests.

What do we change? We could drop down to a single test with the afore mentioned [DataRow] that passes in the currency and values... sure... That hides the smell. It's a single test that can have an ever growing set of inputs. Sure, it works... but it hides silences the tests. We do TDD to help us with our code, but taking actions to silence the code when it speaks to us... we hinder ourselves and our code. We limit the information we get from the code about how it wants to change.

Yes, I anthropomorphize the code, but it so clearly talks to us. I'm not the only one that says this; but it's so true.

Listen to the code. In particular the tests. They are the early warning signals that the design isn't as good as it could be.

We never want to modify the tests first. What if we break the test? We know the system is good with the repetition in the tests. Note: I'm very specifically not calling these tests duplicate or duplication; they both need to exist in the current system.
Once we refactor the repetition in our code, the tests will become duplication, allowing us to refactor them down.

Let's look at our code then

public Money ToWon()
{
    double exchangeRate = _exchangeRates.From(_currency).To("KRW");
    return Money.Won(_amount * exchangeRate);
}

public Money ToDollar()
{
    double exchangeRate = _exchangeRates.From(_currency).To("USD");
    return Money.Dollar(_amount * exchangeRate);
}

What's different?

The currency and created types. Since we're in Money... we don't need to use the creation methods.

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

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

We can extract the currency code repetition into a variable

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

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

What's different now? Just the first line. So let's extract the other two into a method.

public Money ToWon() => To("KRW");

public Money ToDollar() => To("USD");

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

Perfect. Except... This doesn't allow us to reduce the tests. The only way to do that would be expose this method.
That might be a bad idea. Currency is going to spread through our code. That's not great. What's the alternative though?

We add a ToCurrency for every single type of currency that gets supported? That's a lot of work. When all we really need is for the caller to provide the currency. The caller already has to know what they want to convert to so they can call the correct method. Expecting them to use the correct currency isn't that much of a stretch. While we're using string types, it's a huge stretch, we would never want to do that. To easy to make small mistakes that aren't caught.

That's our next requirement anyway. Let's add a new requirement to remove Currency Specific Methods after we change currency from a primitive.

~~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

Before we jump into anything else; let's look at what can be refactored. To get the exchange rate, I copy/pasted the line. Looking at the code here, that line is in 3 places.

Plus]
Equals
To

Let's extract it to it's own method. It's a small thing, but ensures we don't screw up which currency type goes where. Eventually we will. Happens to the sharpest of us.

Or not.. because even the sharpest of us may not have looked too closely...

//Plus
double exchangeRate = _exchangeRates.From(addend._currency).To(this._currency);
//Equals
double exchangeRate = _exchangeRates.From(other._currency).To(this._currency);
//To
double exchangeRate = _exchangeRates.From(_currency).To(currency);

We can see here the easy advantage of using the this. prefix on the currency. It makes it VERY obvious that the orders differ. Without them, it didn't stand out to me.

//Plus
double exchangeRate = _exchangeRates.From(addend._currency).To(this._currency);
//Equals
double exchangeRate = _exchangeRates.From(other._currency).To(this._currency);
//To
double exchangeRate = _exchangeRates.From(this._currency).To(otherCurrency);

While the To method doesn't have an other object, we can name things to help identification. Having the variation, trying to extract anything is going to re-confuse things, which is why we wanted the Fluent methods on the ExchangeRates. So... I'm OK leaving them.

Let's highlight something I've been doing as I'm going through this - I'm making mistakes. My process here is to

write some test
write some code
repeat

You get to see my thoughts and mistakes "real" time. The tests make it safe for me to play around and test some things out. Explore, experiment and if it doesn't work; Go right back to a Green state. Easily.

There is shared behavior, MAYBE it'll be extractable in the future, but now... it'll add complexity, we don't need it. it may even make future opportunities to simplify the code HARDER to see. Early abstraction of things because "we can" and not because "the code wants it" is going to increase complexity and make it much harder to keep the code clean moving forward.

Even with this example; I could extract out the 2 same lines to a method. sure... Then, following the practice of

Make similar things more similar

I'd have to also extract out the non-similar line to a method. Now I've added 2 methods and a 1/2 dozen lines of code to replace 3 lines of code. I've moved functionality further from where it's needed. I've made it possible to invoke the wrong method. This might then cause the method to be modified instead of which one is called. Extracting a generalization early is a setting up maintenance of break things. If it's all in the same method, we can't break the other behaviors. This is only a guide for pieces that aren't being screamed about by the code. Once a generalization is being asked for by the code, we do it. Like creating the ExchangeRates object.
Here, I tried to force the extraction of the lines. The code didn't ask me to. I listen to the code well enough to recognize that it cried out when I started to. I stopped. I didn't do partial because I thought it could help. Nope. The code didn't want it, I didn't do it.
It's hard to get to hear the code. Practicing TDD the way I do it, with the small simple steps help you start to hear the code. The things to have to stand out. Once the things start to stand out, you're starting to hear the code.

This is my favorite bit from my mentor. Listen to the code. it's stuck with me, and I use it every time I want to make changes.

Back to the Code

OK, code is in a rough state right now. There's definitely smells we want to take care of. Definitely things we want to change. Sometimes we need to leave them in place while we fix up elsewhere. In our current situation, we need to get Currency Codes into something other than strings scattered through the system.

NEXT TIME

Technically, we're not done with our current requirement. We don't have a way to convert to Euros

~~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

Don't care. Calling it done. We can see the direction we want to go; using the To method with non-string input. We can see that this is going to remove the ToWon and ToDollar methods. If we added a ToEuro we'd also remove that.

Because this is an exercise, I'm not going to implement the ToEuro method. There are PLENTY of times I implement things I know are going to go way because it's needed to go into production. I've done changes that existed in production for less than 24 hours. Short term solutions are fantastic to meet deadlines. Just make sure they're short term.

Next time, we'll make the changes for the currency and probably have enough time for our last requirement.
I'm looking forward to seeing what the system looks like. This is actually the first time I'm making a Money object with conversions like this. Most money I've had to do in the past stayed single currency, which makes it much simpler.