Testability versus Over-Design?

221 Views Asked by At

Here is the situation actually posed by a co-worker that pegged my interest:

public DoSomething()
{
    //Do Stuff
    var assembly = Assembly.LoadFrom("Path");
    //Do More Stuff
}

So, in order to mock this you have two options

Create an internal virtual method:

internal virtual IAssembly LoadAssembly(String path){...Load Here...}

Or, add a new class that can be passed in

public class AssemblyLoader
{
    public virtual IAssembly LoadAssembly(String path){...Load here...}
}

Both options seem to be a problem as the first seems that it should be a private method, and the second seems to be an over-design of creating a wrapper for a simple static call?

So, I thought I would take it to the community. I am looking for the most pragmatic approach, while remaining unit-testable.

This is similar to this SO question, however I would like to dig deeper into it really.

2

There are 2 best solutions below

2
Dane O'Connor On BEST ANSWER

The question is too general, so it's hard to answer.

To speak generally, I think your problem is artificial. You posit that creating a wrapper for a 3rd party service is over-design, but at the same time say this wrapper is a solution to a real problem. If it solves a real problem, that doesn't sound like over-design, a wrapper actually sounds like good design.

Creating wrappers for 3rd party services is often smart when you need configure state on code you don't control. It doesn't smell as bad as you think. In fact, I don't see another way to do this. No matter how you slice it, whether you're mocking with some 3rd party library, using some reflection magic, or using your purposed solutions, it all boils down to wrapping the real 3rd party api.

If your gut still says wrapping this external api is over-design, maybe you need to re-frame your question. Ask yourself, Should this code be tested?

0
bcarlso On

It's hard to make a generalization here, but the comments before and after loading the assembly(s) suggest that the DoSomething method may be violating the Single Responsibility Principle. That is, the code does too many things. There are a number of ways to write the code such that you'll avoid this, and you mention two of them in your question. I think I'd follow Greg's advice and implement Dependency Injection here. It's difficult for me to say what is and isn't overdesign based on this example.