TLDR in bold
As Jeffery Coffin has already pointed out, there is no one right way to do what you're seeking to accomplish. There is no "one-size fits all" in software, so take all these answers with a grain of salt, and use your best judgement for your project and circumstances. That being said, here's one potential alternative:
Beware of mocking hell:
The approach you've outlined will work: but it might not be best (or it might be, only you can decide). Typically the reason you're tempted to use mocks is because there's some dependency you're looking to break. Extract Interface is an okay pattern, but it's probably not resolving the core issue. I've leaned heavily on mocks in the past and have had situations where I really regret it. They have their place, but I try to use them as infrequently as possible, and with the lowest-level and smallest possible class. You can get into mocking hell, which you're about to enter since you have to reason about your mocks having mocks. Usually when this happens its because there's a inheritance/composition structure and the base/children share a dependency. If possible, you want to refactor so that the dependency isn't so heavily ingrained in your classes.
Isolating the "real" dependency:
A better pattern might be Parameterize Constructor (another Michael Feathers WEWLC pattern).
WLOG, lets say your rogue dependency is a database (maybe it's not a database, but the idea still holds). Maybe MyClass and MyOtherClass both need access to it. Instead of Extracting Interface for both of these classes, try to isolate the dependency and pass it in to the constructors for each class.
Example:
class MyClass {
public:
MyClass(...) : ..., db(new ProdDatabase()) {}; // Old constructor, but give it a "default" database now
MyClass(..., Database* db) : ..., db(db) {}; // New constructor
...
private:
Database* db; // Decide on semantics about owning a database object, maybe you want to have the destructor of this class handle it, or maybe not
// MyOtherClass* moc; //Maybe, depends on what you're trying to do
};
and
class MyOtherClass {
public:
// similar to above, but you might want to disallow this constructor if it's too risky to have two different dependency objects floating around.
MyOtherClass(...) : ..., db(new ProdDatabase());
MyOtherClass(..., Database* db) : ..., db(db);
private:
Database* db; // Ownership?
};
And now that we see this layout, it makes us realize that you might even want MyOtherClass to simply be a member of MyClass (depends what you're doing and how they're related). This will avoid mistakes in instantiating MyOtherClass and ease the burden of the dependency ownership.
Another alternative is to make the Database a singleton to ease the burden of ownership. This will work well for a Database, but in general the singleton pattern won't hold for all dependencies.
Pros:
- Allows for clean (standard) dependency injection, and it tackles the core issue of isolating the true dependency.
- Isolating the real dependency makes it so that you avoid mocking hell and can just pass the dependency around.
- Better future proofed design, high reusability of the pattern, and likely less complex. The next class that needs the dependency won't have to mock themselves, instead they just rope in the dependency as a parameter.
Cons:
- This pattern will probably take more time/effort than Extract Interface. In legacy systems, sometimes this doesn't fly. I've committed all sorts of sins because we needed to move a feature out...yesterday. It's okay, it happens. Just be aware of the design gotchas and technical debt you accrue...
- It's also a bit more error prone.
Some general legacy tips I use (the things WEWLC doesn't tell you):
Don't get hell-bent about avoiding a dependency if you don't need to avoid it. This is especially true when working with legacy systems where refactorings are risky in general. Instead, you can have your tests call an actual database (or whatever the dependency is), but have the test suite connect to a small "test" database instead of the "prod" database. The cost of standing up a small test db is usually quite small. The cost of crashing prod because you goofed up a mock or a mock fell out of sync with reality is typically a lot higher. This will also save you a lot of coding.
Avoid mocks (especially heavy mocking) where possible. I am becoming more and more convinced as I age as a software engineer that mocks are mini-design smells. They are the quick and dirty: but usually illustrate a larger problem.
Envision the ideal API, and try to build what you envision. You can't actually build the ideal API, but imagine you can refactor everything instantly and have the API you desire. This is a good starting point for improving a legacy system, and make tradeoffs/sacrifices with your current design/implementation as you go.
HTH, good luck!
