Still Green; Keep Refactoring
Let's take another look at Money; there's eight methods. Five of these methods delegate the work to another object, "I know what I want, you know how to do it". The remaining three don't. To make "Similar things more similar" we have to consider refactoring these methods into objects. We absolutely can, but I'm not going to.
The reason I'm not is that these three methods all have something similar that's different than the other five methods. They're overrides. These methods override Object methods. In larger systems, I absolutely create objects to do some of this work for me. The Equals method has all the structure required for a generic abstraction
public override bool Equals(object obj) => ReferenceEquals(this, obj) || obj is Money other && Equals(other);
The only thing that'll vary by class will be the type getting cast to. Here it's Money. Will that work as a generic... I actually don't know. I assume a where clause or two will ensure it can be done.
For this example; I'll leave the behavior in Money since there is a distinct difference in the method construction, we'll keep that difference emphasized for now.
Not a lot happening here; but we look and think and sometimes that results in no action. Usually results in delayed action.
Tests Though
Let's look at the tests for money; for this discussion we'll ignore the overrides. You'll see why.
What are tests doing?
The intent behind tests is to ensure the behavior is what we want. We can say that these tests are ensuring the behavior for the Money object is correct.
What behavior?
What behavior in Money is being tested? What does the Plus method do? What behavior does it have?
I don't think these tests are invalid; but they're testing layers of code. They're no longer testing Money; they're testing multiple classes working together. The tests for Plus method will hit multiple classes? Let's count
- abstract Money
- PlusMoney
- ConvertToMoney
- InMemoryExchangeRateCollection
- abstrcat ExchangeRateCollection
- InternalTo
- ExchangeRateOf
- abstract ExchangeRate
- abstract ToSystemType
The test around Money#Plus executes code in 5 concrete classes and 4 abstract classes. We could kick that up if we consider Currency and DefaultCurrency, but I don't think it goes into those, just references.
That's a lot of functionality being executed by a single test, which has become high level. Which is great. We've been refactoring heavily. These tests started off ensuring that the behavior of implementation in Money produced the result we wanted. They still do... they just require a lot of other classes to produce that behavior; which is also great for the system... but not when this test breaks.
Hopefully we have good error messages that will identity what went wrong. The issue with a test that all of the other code that gets executed could break. Our tests should break for one, and only one, reason. We just made a list of 9 reasons our test for Plus might break. That's putting fragility into our system It's allowing classes to exist without tests against their behavior. We want these to be re-usable components in our system, but how can I have confidence that ExchangeRateOf will work when it has no tests around it?
Use it an hope it does since... some other dev probably has tests around the stuff that produced it? What about if the system evolves and the high level tests go away? Maybe Plus gets handed off to an external service. Now there are no tests around the PlusMoney object.
How easy a evolving and ruthlessly refactored system could remove the high level tests make it very risky for the system to not have tests against the class behavior itself.
This leads us into a discussion that will never end - What type of test is it?
TEST TYPE NAME BATTLE ROYAL SUPREME
What kind of tests do we write? Unit tests? System tests? Integration tests? End to end tests? ... WHAT ARE THEY CALLED?!?!?!?
It's a never ending battle. Every team I've worked on has used slightly different terms for the exact same type of tests. I consider Unit Tests any test that operates in the confines of the code. No reaching out to external resources. No Disk, No Network, No Database, ect. These are the tests that could be run by pulling down the code and putting the laptop into airplane mode and running tests. Anything that works is a Unit Test. And... one place I was consulting at... They had tests that passed when they should not have.
Airplane mode is a great way to make sure your Unit Tests don't use external resources. Unit Tests can be divided any number of ways. I typically see them as "micro tests" which will only execute code in a single class. Any other classes that are interacted with are mocked, for interfaces, or use a test only subclass for abstract classes.
That's the only real distinction I care about. If it's not a micro test, it's a code integration test. Some code integration tests are also end to end tests. I love end to end code integration unit tests for validating overall expected behavior. I've used them for webservices and mobile apps. For ma webservice I call the Controller and fake the network responses. Mobile Apps get the same faked network response, but I have to call different entry points. Code execution never relies on external systems, but tests the entire flow of the code, including network failures.
These are nice tests to show the high level behavior of the system and that all the pieces integrate correctly... but are crap for diagnosing and fixing problems. They provide no confidence that any particular class does what it should.
All that being said - We should get tests around all of our classes to provide us certainty that the behavior in the system is accurate for all the pieces of the system.
Once we have these, we'll be able to know that a class we're able to re-use elsewhere does what it says it does; the tests prove it.
As this is the end of what I've got planned - I'm leaving that as an exercise for the reader. Some of these classes will be great candidates for the PrivateCtor class if you're interested in using that. Particularly ConvertToMoney. Using PrivateCtor will allow a test version of the Exchange Rates so values can be test defined.