Class design - Inject data or inject repository/service to fetch data?

Question

Let’s say for example I am developing a fictional class called WidgetMaker. If the class depends at some point on lookup data stored in a database is it better to design the class with a dependency on the data or a service to fetch the data?

Does it make more sense to do this?

public class WidgetMaker {

    public WidgetMaker(LookupData lookupData) {     
    }

    public Widget MakeWidget() {

        if(lookupData.SomeField) { 
            return new Widget("blue"); 
        }
        else { 
            return new Widget("red"); 
        }  
    }
}

-or this-

public class WidgetMaker {

    private readonly LookupDataRepo lookupRepo
    private LookupData lookupData;

    public WidgetMaker(LookupDataRepo lookupRepo) {     
        lookupRepo = lookupRepo;
    }

    public Widget MakeWidget() {

        if(lookupData == null) {
            lookupData = lookupRepo.GetLookupData();
        } 

        if(lookupData.SomeField) { 
            return new Widget("blue"); 
        }
        else { 
            return new Widget("red"); 
        } 
    }
}

The first way seems like the “right” design because we’re injecting what the class needs. It makes the class easier to unit test. Otherwise we need to pass in a mock repo just to test the logic.

On the flip-side, it seems like we're pushing the fetching of data to somewhere else, maybe a WidgetMakerFactory class. Then whatever class that needs a WidgetMaker, would instead get dependency on WidgetMakerFactory which seems like we’re repeating the same issue.

public class WidgetController : Controller {

    private WidgetMakerFactory factory;

    public WidgetController(WidgetMakerFactory factory) {
        this.factory = factory;
    }

    public ActionResult Index() {
        return View(factory.Create().MakeWidget())
    }
}

Answer 1

You should probably start by separating your concerns into small, extensible pieces that can be easily stitched together and replaced.

interface Transform {
    Widget widget(LookupData lookupData);
}

That by itself is a piece

interface Source {
   LookupData lookupData();
}

There's your read side effect.

class WidgetMaker {
    Source source;
    Transform transform;

    Widget widget() {
        return transform.widget(source.lookupData());
    }
}

There's your composition.

class Cache implements Source {
    LookupData cache = null;          
    Source upstream;

    LookupData lookupData() {
        if (null == cache) {
            cache = upstream.lookupData();
        }
        return cache;
    }
}

Now you can put a mini cache in front of any source

... and so on.

it seems you're designing towards injecting the service to fetch the data?

I'm designing toward being able to change my mind. See Parnas 1971.

Injecting immutable data directly into a consumer is going to be my first choice. It's simple, it doesn't entangle the implementation with the imperative shell, which makes the system easier to reason about and easier to test.

But... if the data is something that can be changing during the lifetime of the WidgetMaker, then you have a lot of possible options for how to invalidate your most recently cached copy of the data, and the WidgetMaker itself should be insulated from the strategy that you choose.

When you tell me database, it opens up the possibility that you are needing to access data where the official reference representation is under control of a different process. That's the big scary - needing to access state via the imperative shell, which in the object oriented world means you want to be seriously considering ports and adapters.

Answer 2

My biggest question would be how often the data is expected to change and whether it's supposed to be cached.

Without knowing the answer to that question I'd lean toward option B, injecting an abstraction that will provide the data instead of injecting the data.

If you inject the data, then how will that injection be configured? You could end up with code inside your DI configuration that invokes the repository to provide the data to inject into the class that needs it. I'd rather avoid that and just configure the container to resolve a class. That way the container is only responsible for creating classes, not invoking one to provide data to another.

Also, if you decide to implement caching (like with interception) it's easier to put that around the class that provides the data instead of the class that owns and receives the data. What if a class receives two sets of data, each with different expirations? There would no no way to manage the lifetime of the class to accommodate both. (Not to mention that it's would look really weird to configure a class instance to expire.) Even if there's only one data set it's easier mentally to think of caching the invocation to get the data than caching the class that receives and needs the data.

Along the same lines, what if multiple classes depend on the same data? If you've determined the appropriate cache policy for the source of the data and applied it, then it applies wherever that data is consumed.

If you inject the repository, whether or not you need caching, you don't have to figure all that stuff up front. If you need caching later, you just add it to the repository (ideally using interception.) So to me that's the safest, most flexible option.