What do you look for in a dependency to determine if it should be an injected dependency?

Question

I am having difficult figuring out when a dependency should be injected. Let's just work with a simple example from my project:

class CompanyDetailProvider : ICompanyDetailProvider {
    private readonly FilePathProvider provider;
    public CompanyDetailProvider(FilePathProvider provider) {
        this.provider = provider;
    }

    public IEnumerable<CompanyDetail> GetCompanyDetailsForDate(DateTime date) {
        string path = this.provider.GetCompanyDetailFilePathForDate(date);
        var factory = new DataReaderFactory();
        Func<IDataReader> sourceProvider = () => factory.CreateReader(
            DataFileType.FlatFile, 
            path
        );
        var hydrator = new Hydrator<CompanyDetail>(sourceProvider);
        return hydrator;
    }
}

(Not production quality!)

ICompanyDetailProvider is responsible for providing instances of CompanyDetails for consumers. The concrete implementation CompanyDetailProvider does it by hydrating instances of CompanyDetail from a file using a Hydrator<T> which uses reflection to populate instances of T sourced from an IDataReader. Clearly CompanyDetailProvider is dependent on DataReaderFactory (which returns instances of OleDbDataReader given a path to a file) and Hydrator. Should these dependencies be injected? Is it right to inject FilePathProvider? What qualities do I examine to decide if they should be injected?

Answer 1

I evaluate dependencies' points of use through the intent/mechanism lens: is this code clearly communicating its intent, or do I have to extract that from a pile of implementation details?

If the code indeed looks like a pile of implementation details, I determine the inputs and outputs and create an entirely new dependency to represent the why behind all the how. I then push the complexity into the new dependency, making the original code simpler and clearer.

When I read the code in this question, I clearly see the retrieval of a file path based on a date, followed by an opaque set of statements which don't clearly communicate the goal of reading an entity of a certain type at a certain path. I can work my way through it but that breaks my stride.

I suggest you raise the level of abstraction of the second half of the calculation, after you get the path. I would start by defining a dependency which implements the code's inputs/outputs:

public interface IEntityReader
{
    IEnumerable<T> ReadEntities<T>(string path);
}

Then, rewrite the original class using this intention-revealing interface:

public sealed class CompanyDetailProvider : ICompanyDetailProvider
{
    private readonly IFilePathProvider _filePathProvider;
    private readonly IEntityReader _entityReader;

    public CompanyDetailProvider(IFilePathProvider filePathProvider, IEntityReader entityReader)
    {
        _filePathProvider = filePathProvider;
        _entityReader = entityReader;
    }

    public IEnumerable<CompanyDetail> GetCompanyDetailsForDate(DateTime date)
    {
        var path = _filePathProvider.GetCompanyDetailsFilePathForDate(date);

        return _entityReader.ReadEntities<CompanyDetail>(path);
    }
}

Now you can sandbox the gory details, which become quite cohesive in isolation:

public sealed class EntityReader : IEntityReader
{
    private readonly IDataReaderFactory _dataReaderFactory;

    public EntityReader(IDataReaderFactory dataReaderFactory)
    {
        _dataReaderFactory = dataReaderFactory;
    }

    public IEnumerable<T> ReadEntities<T>(string path)
    {
        Func<IDataReader> sourceProvider =
            () => _dataReaderFactory.CreateReader(DataFileType.FlatFile, path);

        return new Hydrator<T>(sourceProvider);
    }
}

As shown in this example, I think you should abstract the data reader factory away and directly instantiate the hydrator. The distinction is that EntityReader uses the data reader factory, while it only creates the hydrator. It isn't actually dependent on the instance at all; instead, it serves as a hydrator factory.

Answer 2

I tend to be on the more liberal side of injecting dependencies so I would definitely want to inject both IDataReader to get rid of the new DataFactoryReader and the Hydrator. It keeps everything more loosely coupled which of course makes it easier to maintain.

Another benefit that is easy to attain right away is better testability. You can create mocks of your IDataReader and Hydrator to isolate your unit tests to just the GetCompanyDetailsForDate method and not have to worry about what happens inside the datareader and hydrator.