17

Instead of using singletons, I made one class to hold an instance for every component. Let's call it MainApp. MainApp is initialized in the entry point of the program and determines the lifetime of all components and resources. It has a single responsibility and that is being a collection of components. That's why it didn't feel like a bad idea first. However, some components need access to other components, there is one component that is needed for almost all other components. To solve this I used dependency injection, but this makes these components dependent on MainApp. It feels like MainApp just became a big mediator class for all components.

Back to my question: Is this kind of class bad design? What would be better alternatives? Or isn't this class necessarily bad design, is the problem more related to the fact that a lot depends on it?

TheNappap
  • 327
  • 4
    There is not necessarily anything untoward about having an overall "application" object. It's used to great effect in the Microsoft Office suite for example. – Steve Apr 07 '20 at 13:08
  • 4
    I made one class to hold an instance for every component - kinda like an IoC container, then? Something has to know about all the classes in your program and tell them about each other! The thing is, that you sound like you've then hooked your DI up wrong; a class that has a constructor that takes what the class needs doesn't (shouldn't) depend on the IoC container- it should depend on something else and the IOC container should give it that thing, not give it itself. Kinda like, if grandad wants an apple, give him an apple he likes rather than the whole fruit bowl so he can take his own – Caius Jard Apr 07 '20 at 14:17
  • 1
    (Because then you'll get into a protracted discussion about all the different kinds of apples, and oranges, and quinces therein and whether he knows how to eat them, or like them etc) – Caius Jard Apr 07 '20 at 14:18
  • Never heard of an IoC container, but it sounds about the same thing I want to achieve. Nice analogy with the fruit ;) – TheNappap Apr 07 '20 at 14:43
  • 2
    @TheNappap that is a good analogy. It's not that you can't hand grandad the whole fruit bowl. It's that we know the problems that causes. These two different patterns have shown up enough we've given them names: Service locator is the bowl. Dependency Injection is the apple. They solve the same problem but are different – candied_orange Apr 07 '20 at 18:02
  • Honestly, I don't like your design. I like the idea that you have something that could be a dependency injection container, but I don't like the idea that it is injected into other classes. The only reason I can see why you'd ever want to inject this into another class is if you want to decorate it or it's a composable part of a larger implementation. – David Barker Apr 08 '20 at 17:02
  • That's fine, and a very common way of doing things. – GrandmasterB Apr 08 '20 at 17:27
  • "To solve this I used dependency injection, but this makes these components dependent on MainApp." - Sounds like you've rediscovered the Service Locator (anti)pattern. Your MainApp is almost an ad hoc IoC container, except that you don't have a composition root. Instead of referencing MainApp from your components, initialize the dependencies in MainApp and pass them to the appropriate constructors. That way, nothing in your application needs to know about MainApp. – Filip Milovanović Apr 08 '20 at 19:46
  • "Let's call it MainApp" No let's not, let's call it ComponentManager. For the rest you're fine. You may want to implement events on your components and let component manager act as a router for notifications. That way components can communicate without knowing each other. – Martin Maat Apr 25 '20 at 10:52
  • No, I won't call any of my classes "Manager". The name "ComponentManager" is so generic it has almost no meaning. "MainApp" was just an example name and I would never give that name to an any actual class if that makes you feel better. ;) For the event passing idea: it sounds like the micro service architecture, which might work in certain situations, but I don't think it is fit for my use case. – TheNappap Apr 27 '20 at 09:24

4 Answers4

30

This MainApp class is not per se a bad idea, but probably reinventing the wheel.

Generally, you need one "central" component to orchestrate the whole idea of dependency injection. Also, you probably are better off with using a readymade solution (e.g. in Java a Weld container.)

However, the services should not depend on MainApp, but on the other services they need. Let's say, you have a User service which needs a Repository service. Your question sounds like you do this right now:

// BAD
class UserService {
    private MainApp mainApp;

    UserService(MainApp mainApp) {
        this.mainApp = mainApp;
    }

    void doSomethingThatNeedsARepository() {
        this.mainApp.getRepositoryService().doStuff();
    }
}

// in MainApp
UserService userService = new UserService(this);

If you do this, you lose control over which service needs which other service, resulting in a big mess.

Only inject what you actually need:

// Better
class UserService {
    private RepositoryService repository;

    UserService(RepositoryService repository) {
        this.repository = repository;
    }

    void doSomethingThatNeedsARepository() {
        this.repository.doStuff();
    }
}

// in MainApp
RepositoryService repository = new RepositoryService(...);
UserService userService = new UserService(repository);

(We might go on discussing the merits of using an interface instead of a concrete service, but somebody else will probably do this ;-))

Deduplicator
  • 9,031
mtj
  • 2,030
  • 2
    Yep, that's basically the Service Locator anti-pattern in the first snippet. A glorified bunch of global variables stitched together into one. For greater horror, make it static class and give it getWhatever<T>() method: voila, now you have an unbounded amount of global variables and you don't even know which of them exist or not exist. Very handy for breaking borders between isolated parts of your application and turning it into a BBOM. – Joker_vD Apr 09 '20 at 00:02
  • Not to mention the gazillions of lines of Powermockito-code to actually unit-test an isolated little piece of code... – mtj Apr 09 '20 at 11:36
10

However, some components need access to other components, there is one component that is needed for almost all other components. To solve this I used dependency injection, but this makes these components dependent on MainApp. It feels like MainApp just became a big mediator class for all components.

Using MainApp as the Composition Root for Dependency Injection does not make anything else dependant on it unless you're doing something silly like letting the other components know that MainApp exists.

MainApp's job is to construct and inject components. It is not something that components should talk to or ask questions of. Stick to that and your design is fine.

Doing this doesn't make MainApp "represent" your entire program. It's simply how you build its persistent object graph. Once that's done MainApp can call start() on some component and step aside, never to be heard from again.

candied_orange
  • 108,538
  • unless you're doing something silly like letting the other components know that MainApp exists.

    That's exactly what I'm doing... -.-'

    – TheNappap Apr 07 '20 at 13:25
  • MainApp is suppose to start everything and close everything on the end of the program. So for me, it represents the entire program itself. ;) – TheNappap Apr 07 '20 at 13:28
  • For me that’s like thinking the scaffolding represents the building. – candied_orange Apr 07 '20 at 16:07
  • I think the building is not a good analogy here, because a building doesn't really start and stop like a program. I look at it this way: my program is like a (car)engine, you can start and stop it, but you have no idea what goes on on the inside. In the same way MainApp starts and stops the program, but the user doesn't know anything about the implementation. – TheNappap Apr 08 '20 at 07:19
  • 3
    @TheNappap But you're not thinking components there. If you take the engine analogy, candied_orange is talking about what the parts of the engine need to know. Think about a water pump. All it needs to do is pump water. If you bolt extra parts on the water pump to tell it what make of engine it's in, it doesn't help it pump water, and now you've added extra crap to keep in step with the rest of the engine if anything changes, for no good reason. Let the water pump do the one thing it does, and not care about anything else. Then the engine infrastructure takes the water to the right place. – Graham Apr 08 '20 at 18:58
  • @Graham Why would I be thinking about components here, I'm talking about something that represents my entire program. That's not to say that under the hood there a multitude of components which all have 1 responsibility such as your water pump. The engine analogy was just to show why I think MainApp represents my program. – TheNappap Apr 10 '20 at 07:46
  • @TheNappap Because "I made one class to hold an instance for every component." If you're not thinking about how that one class can hook up components with the minimum of cross-coupling, then it will generally end badly. You've already mentioned one child class which everything needs access to, which is a clue that all is not right with your architecture. Good OO design minimises cross-coupling, including back to the parent class unless the child has a really good reason to need it. – Graham Apr 10 '20 at 08:12
  • @Graham Ok, so you are talking about my original question/problem and my design. It's confusing you are answering here. :P I was talking about how the class respresents my program, of course for the implementation of my program I'm thinking how the components are coupled. I know very well that coupling in general should be minimized, but some coupling you just can't prevent. Think about logging, settings, these are often used all over a software piece. In my case, it's an instance of a library object I literally need for every feature. (I'm writing an adapter for this library) – TheNappap Apr 10 '20 at 08:36
5

MainApp is fine. It is a composition root. It should act as glue, injecting dependencies, linking producers and listeners, that kind of stuff.

About the dependency injection, avoid injecting MainApp itself (unless some limitation of the platform forces you). Instead, you can have MainApp inject other things, including factories. If you need to create new types just to inject them, great, do that.

Nothing else should have knowledge of MainApp. A change in MainApp should not imply a change of the components.

Theraot
  • 9,131
  • 2
  • 27
  • 35
2

TL;DR; Injecting your MainApp class loosly couples it to everything it's injected into. Your MainApp class will become the place to solve all sorts of problems because, as you say yourself, it is now a mediator for everything it's injected into. Just inject what is needed for each class, nothing more.

Longer explanation:

It looks to me that you've created an Inversion of Control container or IoC for short. This actually underpins the dependency inversion principle as determined by SOLID architecture. The idea is that a central repository understands how various parts of an application can be constructed and will either hold a reference to an instantiated instance or at least have the ability to construct a new instance with minimal configuration.

Many languages that make use of reflection can actually take this a step further by statically analysing the various dependencies a class needs and then recursively constructing those dependencies based on its knowledge of how they are also constructed. This can be incredibly powerful when building an application as all you need to do is register a class with the IoC and instantly allow the automatic construction of some quite complex dependency trees.

You don't appear to be to far away from this kind of design right now, but you mustn't inject your MainApp class into other classes. This does create dependencies on your registry when you really want to ensure it remains decoupled. Instead you should be looking to construct your classes using the instances of classes it actually needs, not by passing in the registry and letting your classes pull out what they want.

Depending on the language, there are already some excellent IoC implentations available for you to draw inspiration from:

And so on.