3

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.

Community
  • 1
  • 1
Justin Pihony
  • 66,056
  • 18
  • 147
  • 180
  • Couldn't you implement *Dependency Injection*? If you structure it in that manner it will remain both Testable and Maintainable. Though it will be slightly hard to read. – Greg Mar 27 '13 at 18:34
  • The better question is, how much logic are you putting in your constructor? If you need to unit test your constructor it sounds more like a code smell to me. – Ryan Gates Mar 27 '13 at 18:36
  • Voting for the second option. It does create overhead, but it isn't huge, and it serves a "greater purpose". It's clean and comprehensible, and with an auto-registered DI container, you don't even have to care about it any further outside of unit testing. – TeaDrivenDev Mar 27 '13 at 18:46
  • @AlG If you read the full question, I even mention this question and feel that this is not a duplicate. I am asking for a deeper answer. – Justin Pihony Mar 27 '13 at 19:02
  • @Greg The second option is using Dependency Injection – Justin Pihony Mar 27 '13 at 19:03
  • Adding the word deeper doesn't make it a new question. – AlG Mar 27 '13 at 19:13
  • @JustinPihony I didn't read that far, my apologies for quick commenting. – Greg Mar 27 '13 at 20:13
  • 1
    @AlG, using Assembly.LoadFrom *as an example* does not make this a duplicate of a question *that was actually trying to deal with Assembly.LoadFrom.* – Anthony Pegram Mar 28 '13 at 02:28
  • 1
    @JustinPihony, you could probably do some searches over at [Programmers.SE](http://programmers.stackexchange.com), as I believe there have been a few similar topics over there with a diverse range of opinions. I, for one, would go for dependency injection. You know the [adapted] adage, "all problems in computer science can be solved by another level of abstraction... except for the problem of too many layers of abstraction." – Anthony Pegram Mar 28 '13 at 02:38

2 Answers2

3

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?

Dane O'Connor
  • 75,180
  • 37
  • 119
  • 173
  • Yah, I agree. I guess the problem comes from the fact that normally I would not mock a static method. However, this method has external dependencies on the file system. The real problem to me is that this probably should have been an instance object to begin with, and that is the problem...I have to wrap something for the sake of testability and that is why I don't like it. It is changing my code only for the sake of testability, but that is only due to bad design by the 3rd party.... – Justin Pihony Mar 28 '13 at 17:29
  • @JustinPihony "but that is only due to bad design by the 3rd party". Yup, that's exactly the thing you want to abstract yourself from. Changing code for the sake of test-ability is _not_ a bad thing. If you care about testing, changing code to support it is just making your code "more right" given your requirements. – Dane O'Connor Mar 28 '13 at 18:29
0

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.

bcarlso
  • 2,345
  • 12
  • 12