Make Money with TDD - 06
Cross Equality
The requirement we want to get to now is to allow our equality to work across currencies
5 USD * 2 = 10 USD10 EUR * 2 = 20 EUR4002 KRW / 4 = 1000.5 KRW5 USD + 10 USD = 15 USD5 USD + 10 EUR = 17 USD
12 USD == 10 EUR
1 USD + 1100 KRW = 2200 KRW
2.50 USD * 6 = 15 USD
My expectation is that these next few are gonna go QUICK. We've put enough pieces in place that we can just re-use them.
One thing I will do, because it's existing in multiple places; let's add a requirement for Currency types to not be exposed primitives. What the means... I don't know yet
5 USD * 2 = 10 USD10 EUR * 2 = 20 EUR4002 KRW / 4 = 1000.5 KRW5 USD + 10 USD = 15 USD5 USD + 10 EUR = 17 USD
12 USD == 10 EUR
1 USD + 1100 KRW = 2200 KRW
2.50 USD * 6 = 15 USD
Currency not exposed primitives
I use "exposed primitives" as, at some level, it's data. We can't avoid that, but we can avoid the rest of the system having to know, or being able to know about them.
We had the currencies nicely isolated in the Money class, but the extraction of the ExchangeRates definitely got that one spreading out. Gotta contain that somehow.
That's not our now; our now is
A NEW TEST
We need to write a test to confirm equality of different currencies.
[TestMethod]
public void DifferentCurrenciesAreEqualWhenExchangeIsRight()
{
//Arrange
Money tenUsd = Money.Dollar(10);
Money twelveEur = Money.Euro(12);
//Act
bool actual = tenUsd.Equals(twelveEur);
//Assert
actual.Should().BeTrue();
}
I definitely don't like the reliance on the exchange rate in these tests. I could use something to do magic and get the exchange rate, and then create a new amount... bleh. It's a lot of work for what will be done via the constructors. So... Not now.
I'll continue to let the coupling to the exchange rates exist; recognizing it does make the tests more fragile.
This test fails... cause... duh. We haven't implemented it yet. Which; AWESOME!
Now we need to write some code to make it pass.
... Did you notice I messed up which numeric value went to the dollar and euro... Because I sure didn't.
Which, when correct, makes the test pass.
This, unfortunately, creates a problem... One of the other tests validating that different currencies of the same numeric value are not equal fails. USD and KRW don't have an exchange rate, for example.
[TestMethod]
public void DifferentTypesAreNotEqual()
{
//Arrange
Random random = new();
int value = random.Next(1, 20);
Money euro = Money.Euro(value);
Money won = Money.Won(value);
Money usd = Money.Dollar(value);
List<Money> shuffled = new List<Money>
{euro, won, usd}
.OrderBy(a => random.Next())
.ToList();
//Act
bool actual = shuffled[0].Equals(shuffled[1]);
//Assert
actual.Should().BeFalse();
}
How should we handle this test? Its no longer implemented in a valid way. The test itself may no longer even be valid. There's a test that validates an exception thrown for missing exchange rates.
There's a test that makes sure equality across currencies is happening... Maybe a test that's more limited to showing that cross currency that isn't right aren't equal... like the first version of my "are equal" test?
I think that's the best option. Remove this, as it does nothing, and add an invalid equality for now. Once my magic utility gets pulled in, we can decouple the tests from the implemented ExchangeRates.
[TestMethod]
public void DifferentCurrenciesOfSameValueAreNotEqual()
{
//Arrange
Random random = new();
int value = random.Next(1, 20);
Money euro = Money.Euro(value);
Money usd = Money.Dollar(value);
//Act
bool actual = usd.Equals(euro);
//Assert
actual.Should().BeFalse();
}
There needs to be some care when comparing values that we want to get a result from. EUR does not exchange to USD. My first pass had euro.Equals(usd) and that was throwing an exception.
Once we have it correct, it passes. We're all green
Equals
That's our requirement complete.
5 USD * 2 = 10 USD10 EUR * 2 = 20 EUR4002 KRW / 4 = 1000.5 KRW5 USD + 10 USD = 15 USD5 USD + 10 EUR = 17 USD12 USD == 10 EUR
1 USD + 1100 KRW = 2200 KRW
2.50 USD * 6 = 15 USD
Currency not exposed primitives
I was pretty sure that was gonna go fast. And ... very much so. Let's see if there's some refactoring we want to do before we move on.
To equal or not to equal, that is the hashcode
Oooo... Here's an issue - The hashcode will not be equal when the objects Equals says they are. That's actually exceptionally disastrous...
How can we solve this?
Can't delete the method... it'll still be there, just even worse...
Can't convert everything to a single type of currency... that's horrible maintainability and not everything converts...
Well... if it doesn't convert...
Well... shit... I'm not sure how to solve this one.
Does it need to be solved??
Let's think about it...
Are the objects equal... yes. 12 USD is equal to 10 EUR.
Is 12 USD the same as 10 EUR? No. If I wanted to put 12USD into a collection and 10 EUR into a set, They should be distinct. If I put 12 USD and 12 USD into a set... it already exists in there, so shouldn't be added again.
I'm not comfortable with it. Depending on the broader picture, I'd try to find information to help drive a better answer. Right now... Since I don't have a fix to get different currencies the same hash... we'll accept that different currencies do not have the same hash.
... Feels very wrong.
Next Up
Let's move onto our next requirement, which appears to be just a new exchange rate.
5 USD * 2 = 10 USD10 EUR * 2 = 20 EUR4002 KRW / 4 = 1000.5 KRW5 USD + 10 USD = 15 USD5 USD + 10 EUR = 17 USD12 USD == 10 EUR
1 USD + 1100 KRW = 2200 KRW
2.50 USD * 6 = 15 USD
Currency not exposed primitives
What is this? Money for ants?
I think this one is going to require two tests. One around the ExchangeRates to make sure it's got the values, then one against the Money to make sure we have the right currencies and they exchange properly.
Feels a little coupled... Which has been mentioned before. These tests will get a pass when the requirements are in place. For now, let's create a conversion.
[TestMethod]
public void ExchangeUsdKrw()
{
//Arrange
ExchangeRates subject = new ExchangeRates();
//Act
double actual = subject.ExchangeRate("KRW", "USD");
//Assert
actual.Should().Be(1100);
}
Compiles, tests run, it fails. My bet is that it fails for the No Exchange Exception.Test method MoneyExample.ExchangeRatesTests.ExchangeUsdKrw threw exception: MoneyExample.NoExchangeRateAvailableException
Perfect.
Let's go make it pass!
Pretty easy modification of the static exchange rates
{ "USD", new()
{
{ "USD", 1.0 },
{ "KRW", 1100 }
}
}
And the test passes!
Refactor
I keep messing up the parameter order for the exchange rate method. It's kinda annoying. I feel like a fluent interface for this object would fit much better.
exch.From("USD").To("KRW")
It'll be MUCH more obvious what's the right way.
Let's refactor this in.
Not bad... couple interfaces, a private class
private class InternalTo:IExchangeRateTo
{
private readonly Dictionary<string, double> _map;
public InternalTo(Dictionary<string, double> map)
{
_map = map;
}
public double To(string currency)
{
if (!_map.ContainsKey(currency)) throw new NoExchangeRateAvailableException();
return _map[currency];
}
}
The throwing logic has now been split into 2 places. One as part of the ExchangeRates new From method, and the other seen above. There's only a single test for throwing the exception... We need to add another test to cover the split.
The existing one covers the throwing seen above.
[TestMethod]
public void NoExchangeRateIsException()
{
//Arrange
ExchangeRates subject = new ExchangeRates();
//Act
Func<double> actual = subject.Invoking(x => x.From("USD").To("BLEEP_BLOOP"));
//Assert
actual.Should().ThrowExactly<NoExchangeRateAvailableException>();
}
We'll add another that tries to get the exchange rate from BLEEP_BLOOP.
[TestMethod]
public void NoFromExchangeRateIsException()
{
//Arrange
ExchangeRates subject = new ExchangeRates();
//Act
Func<IExchangeRateTo> actual = subject.Invoking(x => x.From("BLEEP_BLOOP"));
//Assert
actual.Should().ThrowExactly<NoExchangeRateAvailableException>();
}
I didn't have to do this refactor. But it wasn't an intuitive API. I just wrote it, and I was getting it wrong! The code is letting out a deafening scream that it needs to be made safer!
Listen to the code, it will help you get to the right place. Now, we can easily see if our conversion is going the way we expect it to, or not. Where as before, it was a mystery unless you went into the method.
Self documenting code is one of the great ways to reduce errors of ambiguity.
With that, our new exchange rate is in. DONE! I didn't expect that to take much... and it surprised me with how little it took. Even with the fluent addition... just breezed through.
Next Up... again
5 USD * 2 = 10 USD10 EUR * 2 = 20 EUR4002 KRW / 4 = 1000.5 KRW5 USD + 10 USD = 15 USD5 USD + 10 EUR = 17 USD12 USD == 10 EUR1 USD + 1100 KRW = 2200 KRW
2.50 USD * 6 = 15 USD
Currency not exposed primitives
Hah... yeah... this is in place. We're just validating we can pass in a double. ... Since I don't want to write a test that's gonna pass... I'll modify a test.
Our trusty addition test shall become a double
[TestMethod]
public void FiveDollarsPlusTenEuroShouldBeSeventeenDollars()
{
//Arrange
Money fiveDollar = Money.Dollar(5.5);
Money tenEuro = Money.Euro(10);
//Act
Money actual = fiveDollar.Plus(tenEuro);
//Assert
actual.Should().Be(Money.Dollar(17.5));
}
There's no reason to add another test to it. It's not terribly a test that needs to be so explicit. I like having non-ints as input to non-int fields to make sure we don't mess things up.
Calling this one done... even quicker than I thought.
5 USD * 2 = 10 USD10 EUR * 2 = 20 EUR4002 KRW / 4 = 1000.5 KRW5 USD + 10 USD = 15 USD5 USD + 10 EUR = 17 USD12 USD == 10 EUR1 USD + 1100 KRW = 2200 KRW2.50 USD * 6 = 15 USD
Currency not exposed primitives
Next Time
We've burned through some requirements in this section; We'll keep the refactoring the currency strings to something else for the next section. Not sure what that's gonna do, but we'll have the space to do it in.
Though, a thought has occurred to me - We have no way to convert an amount of a currency into another currency. ... Seems a bit of an oversight.
Sure we could create 0 USD and add 1100 KRW to get 1 USD, but that's hacky as hell. We should get a proper way to do it added to the requirements.
5 USD * 2 = 10 USD10 EUR * 2 = 20 EUR4002 KRW / 4 = 1000.5 KRW5 USD + 10 USD = 15 USD5 USD + 10 EUR = 17 USD12 USD == 10 EUR1 USD + 1100 KRW = 2200 KRW2.50 USD * 6 = 15 USD
Convert between currencies
Currency not exposed primitives
Excellent! I don't expect that will be terribly complex, all the required components are in place; but I think it'll open up some refactoring opportunities. I'm looking forward to the discussions of why I want to do each of those refactorings.