Get dependencies from a dependency

Question

Let's say I have a PetManager and a Cat:

class PetManager
{
    PetManager(IBusinessLayer businessLayer, IWashingService washingService);

    IBusinessLayer BusinessLayer;

    IWashingService WashingService;
}

class Cat
{
    Cat(PetManager manager, string name, int levelOfStupidity);
}

Now let's say that my cat needs the washing service, would it be so baaaaad, to get the dependency from my pet manager ?

class Cat
{
    Cat(PetManager manager, string name, int levelOfStupidity)
    {
        this.manager = manager;
        this.name = name;
        this.levelOfStupidity = levelOfStupidity;
    }

    IWashingService WashingService
    {
        get { return this.manager.WashingService; }
    }
}

I strongly suspect that yes, it would be...

score 4 · Accepted Answer · answered Apr 26 '11 at 15:18

4

As stated, Cat is a concrete class, so it can expose whatever makes sense. Exposing constructor arguments as read-only properties is a perfectly sensible thing to do.

However, if Cat implemented ICat I would strongly suspect that exposing a dependency like PetManager through ICat would be a leaky abstraction.

In essence, interfaces serve as a sort of access modifier. It makes sense to expose dependencies on the concrete class, but not on the interface. Dependencies are injected through the constructor so can never be part of the interface - the constructor signature is our degree of freedom.

answered Apr 26 '11 at 15:18

Mark Seemann

225,310
48
427
736

+1! Why would a cat be exposing a pet manager?!? That's not a property of a cat! – Domenic Apr 26 '11 at 15:30
Thanks for the interface explanation, it's crystal clear. The PetManager property is not really one, it's more like a proxy to retrieve the wanted dependency. Saying that, basically what i have done here, and I quote brian, is "abstracting a bunch of related dependencies into another class and just passing that through as a way of grouping and perhaps simplifying object construction". How it is compatible with dependency injection ? – Roubachof Apr 26 '11 at 15:32
1

+1 because Mark literally wrote a book on this, and it probably didn't him rich, so he at least deserves a vote. – Brian MacKay Apr 26 '11 at 15:51
@Roubachof: It might be okay to do that, but I'd be wary of it. Not because it conflicts with DI (it might or it might not depending on details not yet provided), but just because it sounds like it might have too many responsibilities. Those things quickly become unwieldy. HttpContextBase comes to mind as an excellent example of a problematic design. – Mark Seemann Apr 26 '11 at 17:31
1

I know what you mean. I realized recently that some of my high level classes were breaking the SRP. It's often the case when the word 'manager' come to the table. It seems it's the accepted terminology for a swiss army knife. – Roubachof Apr 27 '11 at 08:21
@Roubachof: Hear, hear. So in a situation where you had, say, a CustomerManager object that contained dozens of static methods like CreateOrder and CheckDiscounts and so on, how would you set that up to maintain SRP? Just lots of little classes? I kind of like being able to know that it's all in one place. – Brian MacKay Apr 27 '11 at 14:54
@Brian: I think the issue appears when you cannot answer clearly the question "what is the purpose of this class ?" If you find yourself enumerating distincts behaviors, or often worse, if you have to actually take a look at its implementation to answer it, something must be wrong. In fact, you should be able to answer to the question just by knowing its name. This is why manager seems wrong to me, it's vague and doesn't match a well-known terminology. Most of the time, managers are mainly repositories merged with others behaviors. – Roubachof Apr 27 '11 at 15:28
I just found this post which pretty much wraps it up: http://stackoverflow.com/questions/1866794/naming-classes-how-to-avoid-calling-everything-a-whatevermanager – Roubachof Apr 27 '11 at 15:31
@Roubachof: Hey thanks. I tend to have Manager classes in my code in situations where there are a bunch of small bits of business logic, usually less than 10 lines to a method, that seem too simple to merit a seperate class for each of them. I think if there were, say, 20 tiny classes instead of one big one it would be the same situation, except the clutter would be in a namespace instead of a class. But it probably is more correct to have MyProject.Customers.OrderPlacement.PlaceOrder instead of just MyProject.CustomerManager.PlaceOrder... Meh, I honestly think it doesn't (cntd) – Brian MacKay Apr 27 '11 at 16:23
@Roubachof: matter much as long as the code remains maintainable. I suppose you could also have a CustomerManager that makes a call out to the OrderPlacement class -- I do that whenever things start to get a bit more complex. I have to admit, I feel like I'm missing something here. – Brian MacKay Apr 27 '11 at 16:25
@Brian: It will become difficult to discuss this point here as we'll have to go deeper into examples. The granularity of the SRP is the debate. I previously tend to group business classes by entity with name like Business, which ones have a relation to Repository. The repositories contain only linq to entities requests and are part of the DAL. With your example it will be more like. MyProject.OrderBusiness.AddOrder and MyProject.CustomerBusiness.Xxx. Anyway, what I am really sure of, is that we are happily messing up the comments thread :) – Roubachof Apr 27 '11 at 17:08
@Roubachof: Yeah that's what my data objects look like. Well... I guess maybe we should delete all this. :) – Brian MacKay Apr 27 '11 at 18:52

score 1 · Answer 2 · answered Apr 26 '11 at 14:46

I guess that depends. Often times, when you find yourself having a SomethingManager class, you're just grouping logic into one class instead of splitting it into it's constituent dependencies. In your situation, it seems you really shouldn't have a PetManager class at all, and instead be injecting the dependencies for your WashingService and BusinessLayer objects directly.

Brian MacKay · Answer 3 · 2011-04-26T15:45:33.380

Well, if you're subscribing to the inversion of control/dependency injection style (and it seems like you might be), you have to think about the trade-offs.

I guess what the diehards might say that you could get some maintenance problems from this. They certainly do not seem squeemish about just having tons of parameters. So for instance, if you used PetManager on 10 different kinds of pets, and one of those ten pets needed some special functionality that caused PetManager to change, you could impact the other 9 classes that depend on PetManager, and therefore would have been better off just injecting the dependencies individually.

Being pragmatic though... What you're doing is abstracting a bunch of related dependencies into another class and just passing that through as a way of grouping and perhaps simplifying object construction. I'm okay with it. I even kind of like it.

Full disclosure though: I'm not as diehard about this as some other people. I may well be a heretic, but fewer parameters looks and smells cleaner to me. I have this nagging sense that if ask me again in five years I may feel differently, but that's where I'm at right now.

score 1 · Answer 4 · answered Apr 26 '11 at 15:00

1

I think the only problem is that cat depends on a concrete Petmanager, if you abstract PetManager as a service, able to provide a washing service, it sounds better to me.

answered Apr 26 '11 at 15:00

Felice Pollano

32,832
9
75
115

Get dependencies from a dependency

4 Answers4