0

Imagine the following constuctor

public RestClient(
    string key,
    Uri baseAddress,
    IHttpClient httpClient,
    ISerializer serializer,
    IResponseFactory responseFactory)
{
    _key = key;

    _baseAddress = baseAddress;
    _httpClient = httpClient;
    _serializer = serializer;
    _responseFactory = responseFactory;
}

When is it appropriate to create a local default like this?

public RestClient(
    string key,
    Uri baseAddress,
    IHttpClient httpClient,
    ISerializer serializer,
    IResponseFactory responseFactory)
{
    _key = key;

    _baseAddress = baseAddress;
    _serializer = serializer;
    _responseFactory = responseFactory;

    HttpClient = new HttpClientAdapter();
}

IHttpClient HttpClient {get; set;}

and allowing the dependency of IHttpClient to be overridden using property injection.

Or is it the responsibility of the consumer to pass the dependency in the constructor and not rely on local defaults at all? Mark Seemann, in his book describes that property injection is appropriate when there is a good local default but I am struggling in this example to decide whether or not it is appropriate to use a local default in the first place.

maccettura
  • 10,514
  • 3
  • 28
  • 35
user3154431
  • 308
  • 3
  • 8
  • 1
    I think the entire point of dependency injection is to _never_ new up a dependency in the constructor/code. Also, your second example _completely_ ignores the passed in `httpClient`, that seems like a bad thing to me – maccettura Apr 04 '18 at 21:50
  • 1
    You *could* offer a second constructor that doesn't accept an IHttpClient....but why would you? Why not just resolve it via DI? You're just asking these questions hypothetically I presume....but is there some real world problem you're trying to solve? – mason Apr 04 '18 at 21:52
  • It seems very odd to pass in `IHttpClient httpClient` and then do nothing with it. In limited instances I have seen `_httpClient = httpClient ?? new HttpClientAdapter();` as a useful _transitional_ stage if you are adding a new dependency to a class that (due to history) is manually newed up in many places. It allows you to use constructor based DI, easier unit testing and yet also provides a 'fallback' until you have gone and changed all of the calling code. – mjwills Apr 04 '18 at 21:59
  • This example is purely hypothetical. I am asking when it is appropriate is supply a local default and when it is better to not do this at all and purely rely on DI alone and not allow defaults to be overridden. – user3154431 Apr 05 '18 at 08:28

1 Answers1

0

I would never have local defaults like that, as it is a pain to unit test.

If you would like to simplify instantiation of the class, using the default class, then I would go instead with an constructor override like this:

public RestClient(
    string key,
    Uri baseAddress,
    ISerializer serializer,
    IResponseFactory responseFactory)
    : this(key, baseAddress, new HttpClientAdapter(), serializer, responseFactory)
{
}

public RestClient(
    string key,
    Uri baseAddress,
    IHttpClient httpClient,
    ISerializer serializer,
    IResponseFactory responseFactory)
{
    _key = key;

    _baseAddress = baseAddress;
    _serializer = serializer;
    _responseFactory = responseFactory;

    HttpClient = httpClient;
}

IHttpClient HttpClient {get; private set;}
jonasm
  • 977
  • 1
  • 8
  • 14
  • I've recently been under the impression that multiple constructors are bad. https://www.cuttingedge.it/blogs/steven/pivot/entry.php?id=97 – user3154431 Apr 05 '18 at 08:25
  • It depends on what you need. If you run full dependency injection, there's no need for the first constructor in my example, as all is setup when setting up your dependencies. If you need to new up the class without dependency injection, then a constructor override is the best choice. – jonasm Apr 05 '18 at 10:26