Money Splits
And we're back! Let's remind ourselves (yeah yeah... me) of our requirements
5 USD * 2 = 10 USD10 EUR * 2 = 20 EUR
4002 KRW / 4 = 1000.5 KRW
5 USD + 10 EUR = 17 USD
1 USD + 1100 KRW = 2200 KRW
2.50 USD * 6 = 15 USD
And now we're doing division of ... South Korean Won.
Print It
The first thing to do is my wonderful Exists path.
For this, I'll use the MoneyTests.cs file and add a WonTests class. I create the new class in an existing file because the tool will move it for me.
With it in it's own file, we can start creating the class
[TestMethod]
public void WonExists()
{
new Won(5);
}
This gives us the compilation failure we so desired, and a quick hot keys will create the class for us.
ReSharper
One of my favorite things with ReSharper is ALT+Enter, Enter. This is the hot keys to do, usually, the right thing. ALT+Enter brings up the quick action menu and Enter will do the top item. This is 95% of the time, the thing you want. Sometimes it's the 2nd or 3rd item in the list you want, but easy to undo and go pick the right thing.
It's some nice muscle memory that allows generating a lot of code.
But Money
Our Won class doesn't extend Money. I could have defined that in the first test and it would have generated it for me. That's OK, it's not the test we were writing, but let's.
[TestMethod]
public void WonInheritsMoney()
{
Money subject = new Won(5);
}
This test fails to compile; perfect!
... Hmmmm... Sometimes... sometime the Resharper quick menu fails me. No option to add the inheritance.
Well... phoeey... Guess I now know to go ahead and add the base class to the create next time. Otherwise I gotta write some code...
public class Won:Money
{
public Won(int amount):base(amount, "")
{
}
}
Our Won now looks like the above. I didn't add the currency code because we'll be forced to with another test.
It's arguable if we should have passed in the amount or not... Meh... it's a variable traveling, we had to put SOMETHING, I don't think it's outrageous to put that in. We're some sharp developers, we can see where this is going.
Is it Right?
I could have passed in 0 instead of amount and then written a test to confirm that value is correct. Probably would have been better to do that, even though it'd get deleted. It's a test to force the code into a very specific shape.
Since we've already got a commit in with the amount being based to the base constructor, I'll leave it. But it's good to think about how we can take smaller steps and better force the code to evolve. Getting good at tests for these very small steps, and using the tools to generate most of the code, you'll be just as fast as writing the code in the first place.
I've found that doing these small steps actually make me more productive overtime as it makes sure I'm doing ONLY what's needed. Like the empty string being passed in.
String it along
And now we need to make sure we have the currency... and the amount because I didn't use a test to force that.
What? You ask why I can't write that test now? Absolutely.
What is the TDD loop?
Red-Green-Refactor.
What value is there in a test we never see fail?
None*.
Because we already pass in the amount, any test to verify it won't fail, and we can't do RGR. That's not to say that "I want a test to prove my understanding" and to serve as documentation are bad; I will write those all the time. I want to BE SURE that something works and be able to document that it works. They are valid reasons to write a test, but it's not TDD.
Since the next test I'm going to write will use the amount, I don't think it's a big deal to not have the test that would be deleted.
Thanks for asking.
And now the test
[TestMethod]
public void AsStringReturnsValueAndCurrency()
{
//Arrange
Money subject = new Won(9);
//Act
string actual = subject.AsString();
//Assert
actual.Should().Be("[amount=9][currency=KRW]");
}
This will make sure our amount is correct and force us to put in the correct currency code. This is a great place to remind ourselves that we should check the test failure output. We've made a little bit of a bit step; and we expect it to fail solely because of the missing KRW. Anything else means we're misunderstanding something.
Expected actual to be "[amount=9][currency=KRW]" with a length of 24, but "[amount=9][currency=]" has a length of 21, differs near "]" (index 20).
Phew. We're missing the expected value and that's why it failed. An easy fix.
public class Won:Money
{
public Won(int amount):base(amount, "KRW")
{
}
}
And we're all green!
Refactor Won Out
If you're following along in the code, you may have noticed that I've put both the Dollar and the Euro classes in the Money.cs file. I tend to do this with knowledge classes. They are too small to justify their own file, and are very tightly associated with the base class.
It's time to move the Won into here.
I don't know how to make the tooling do that for me. So... COPY PASTE!!!
And now all 3 currency classes are together with Money.
Right; ok. Won exists. Which is only PART of our current requirement.
5 USD * 2 = 10 USD10 EUR * 2 = 20 EUR
4002 KRW / 4 = 1000.5 KRW
5 USD + 10 EUR = 17 USD
1 USD + 1100 KRW = 2200 KRW
2.50 USD * 6 = 15 USD
We need division.
Where to divide?
While we could add the division code onto Won itself... I think that's adding extra work for us down the line. Unsafe work. We're sharp, we know we'll want the same thing for all money types... which means it should probably go on Money; let's do it there.
This means a new test on Money to make Divide exist
[TestMethod]
public void DivideExists()
{
Money subject = new TestMoney(4002, "CUR");
double actual = subject.Divide(4);
}
I bet you thought I'd forget to do my Exists test. You were right. Then I fixed it.
Quick hot keys gets me a default method that passes all tests
public double Divide(int i)
{
return 0.0;
}
Since we're green, and i sucks... Let's refactor it to divisor
Let's Split
And now we divide!
[TestMethod]
public void DivisionOf4002By4ShouldBe1000_5()
{
//Arrange
Money subject = new TestMoney(4002, "CUR");
//Act
double actual = subject.Divide(4);
//Assert
actual.Should().Be(1000.5);
}
Mostly. It's our failing test.
Which is SUPER easy to get passing
public double Divide(int divisor)
{
return 1000.5;
}
I'm gonna copy/paste our test, and make changes from the bottom up to split 10,000 into 1,000.
With a failure for just the right reason Expected actual to be 1000.0, but found 1000.5.. Which can be updated to pass with ease...
public double Divide(int divisor)
{
if (divisor == 10) return 1000;
return 1000.5;
}
OH YEAH!
Being the sharp developers we are, we've seen this kinda pattern before. And yes... writing about writing triangulation tests... sucks about as much as reading about writing triangulation tests. It's still good practice to do them. It definitely helps drive home the value of the tools doing code generation for you. Copy/paste is always good and that'll help drive the "bottom up" changes to a copy/paste test.
When I add my "fix"
public double Divide(int divisor)
{
return _amount / divisor;
if (divisor == 10) return 1000;
return 1000.5;
}
There's a failing test... Let's check the reason Expected actual to be 1000.5, but found 1000.0.Well... That would be weird if we weren't sharp developers that know about type casting and truncation and if you use an int in an operation, the result is an int.
We're red. Our refactor failed. Let's undo it. Which... golly-gee... just requires deleting the new line. When we get rid of that top line, we're back to green.
Refactor Safely
This is a great (simple) example of why I add the line at the top before removing the triangulation code. I can revert in an instant and KNOW I'm back to a good state. And we can be sure that we didn't accidently break something in the pre-existing code.
ReSharper actually warned me that there was a potential loss of fraction in that line. Pay attention to the tools, they're pretty darn smart.
I've updated the line to convert our _amount to a double
public double Divide(int divisor)
{
return Convert.ToDouble(_amount) / divisor;
if (divisor == 10) return 1000;
return 1000.5;
}
Which keeps us green. Is this a perfect solution? Nooooo. _amount should be a double.
Which... we're at a place I think it's a fine refactor to do.
Refactor Signature
We can use our tool to modify the signature of the constructor of Money to change amount from an int to a double.
Which is a cascading change of types. Fortunately we can hard cast each one into a int to take it a small step at a time.
Working through each one, making sure we stay green. The reason we want to take these one at a time is that our tests require a change. A couple tests get the return type of an int and don't compile if they were getting a double out.
This is one of the reasons I prefer not to use var. The type explicitness helps make sure the changes I'm making are REALLY the ones I want to be making.
Interestingly... Nothing here forced us to change the signature of our derived classes..
OH NO! I missed a huge thing! AHHHHHH!!!!
OK, We're green, I'm gonna commit our Money amount type change.
Abstract or ____ Nothing inbetween
Look at the signature of our derived classes public class Dollar : Money. It's... open! Types should be abstract or sealed. I don't think classes should be able to be instantiated AND inherited. It's too much risk, too much coupling, too much of a headache.
This changes to public sealed class Dollar : Money. It's one of the things really like about Kotlin. Classes are "sealed" by default. (Yeah yeah, final in java terms)
OK... phew. Glad I caught that. It would have had no impact on anything if it stayed!
Double Up
We can take a couple paths to get our classes to be double types. Could just update it. But it could always revert. We need a test for each.
... Glad I started with the Won... Apparently I forgot to clean up the duplicate tests. I'll go ahead and do that now while we're on green.
The test we need just adds a decimal component to the constructor value
[TestMethod]
public void WonTakesDouble()
{
//Arrange
Money subject = new Won(9.6);
//Act
string actual = subject.AsString();
//Assert
actual.Should().Be("[amount=9.6][currency=KRW]");
}
We could have modified the AsString test, but then we're not writing a new test, we're changing and existing one... which breaks. That's scary. Write new tests to get to Red, then refactor when on Green.
ReSharper will make the type change for me from the test. It's slick. public Won(double amount) : base(amount, "KRW"){}
Our two tests in WonTests are now almost identical. We can probably get rid of the non-double one. I'll be tricksy and now update the int value to a double value so I have to type less. Renaming is hard.
For the Euro and Money, we ... can short cut since we saw it work for the Won. I'm ok with that. Since I'm modifying an existing test, following a demonstrated successful flow, I'm going to do a single commit for the test update and code fix to keep things green.
While we are taking a shortcut of the tests we're writing for Euro and Dollar - It's critical to understand that we're skipping steps because we PROVED those steps get us to this final state. We're using that proof immediately after. If we delayed for a bit and came back to make the change to Dollar... I wouldn't be OK skipping the steps. We can't be sure what else has happened. We need the confidence of those additional test steps to ensure functionality is maintained. Changing a test is a huge risk to the system. Do not do it lightly.
Refactor?
Pretty sure we're done with out latest requirement
5 USD * 2 = 10 USD10 EUR * 2 = 20 EUR4002 KRW / 4 = 1000.5 KRW
5 USD + 10 EUR = 17 USD
1 USD + 1100 KRW = 2200 KRW
2.50 USD * 6 = 15 USD
While we never wrote the test for division in KRW, we know that it works from the tests in Money. We're OK with this requirement being called done.
Going through the code, one thing stands out to me... the Value method. What's it there for? Nothing uses it. What happens if we delete it.
One of our first tests fails to compile
[TestMethod]
public void MoneyReturnsConstructedValue()
{
//Arrange
int value = new Random().Next(1, 20);
Money subject = new TestMoney(value, "CUR");
//Act
double actual = subject.Value();
//Assert
actual.Should().Be(value);
}
Which... OK... Let's go ahead and remove that test since it doesn't seem to be needed.
And we're green again. We no longer need the method. Fantastic, less code.
This one really isn't just "less code". This get to the fundamental of what I think objects are - They are units of Associated Behavior. Returning a primitive value isn't a behavior. Normally whatever gets that value will DO something with it. I bet our object can do that "something". If it's needed in one place, it's probably needed in a few places, so let's keep that behavior in a single place with a single implementation.
One of my biggest principles is "No Getters". They show up early on in object creation, but we need to find ways to remove them. We are currently getting the same validation of object behavior through the AsString method. Is it great... nah... but it works.
More Refactor
And as I glance through the code... I realized I'm a bit late on an idea... It's done MUCH earlier in both books. ... Let's clean up a bit more first. I still have the DivideExists method, which... don't need that.
And Money has 2 hard coded Division tests... We can switch that to randomization.
Now that we have the double type coming into our Money constructor, we no longer need to convert to a double for our Divide method.
public double Divide(int divisor)
{
return _amount / divisor;
return Convert.ToDouble(_amount) / divisor;
}
All our tests stay green, so good to clean up.
Missed Something Big
I missed something big in our requirements. In my defense, it was past 1am.
The requirements say
5 USD * 2 = 10 USD10 EUR * 2 = 20 EUR
4002 KRW / 4 = 1000.5 KRW
5 USD + 10 EUR = 17 USD
1 USD + 1100 KRW = 2200 KRW
2.50 USD * 6 = 15 USD
and the values I get back for multiplication and division are... just a value. They aren't in any currency. OMG!!!
I dun' fucked that one up. I haven't mentioned it yet, but this is why I really miss ensemble programming. A group of us would have never missed that. Even at 1am.
To save you some reading... I'll take care of that next time.
NEXT TIME!
Fixing my fuckup. Let's get money back, not just amounts.