1

I understand that having 3 or more parameters as dependency is a sign that the class may be doing too many things, however there are cases that the class really needs more than 3. My question is, is consolidating the dependencies in to just one parameter an acceptable practice?

E.g.,

Instead of Foo needing to instantiate and pass its dependencies like this:

$dep1 = new Dep1;
$dep2 = new Dep2;
$dep3 = new Dep3;

Foo($dep1, $dep2, $dep3);

all it needs to do is something like this:

$dependencies = new foosDependencies;
Foo($dependencies);

foosDependencies looks something like this:

class foosDependencies
{
    public $dep1;
    public $dep2;
    public $dep3;

    public function __construct()
    {
        $this->dep1 = new Dep1;
        $this->dep2 = new Dep2;
        $this->dep3 = new Dep3;
    }
}

and Foo like this:

class Foo
{
    private $dependencies;

    public function __construct(foosDependencies $dependencies)
    {
        // Foo now has access to the ff.
        $dep1 = $dependencies->dep1;
        $dep2 = $dependencies->dep2;
        $dep3 = $dependencies->dep2;
    }
}

Now if ever you need more dependencies, you just add it in foosDependencies without the need to modify the constructor parameters of Foo.

Is this is an acceptable method?

  • "however there are cases that the class really needs more than 3" Then there's no problem with a constructor having more than three parameters. It's a guideline, not a blood pact! – Vincent Savard Jul 10 '19 at 12:38
  • Can you ensure all instance will always initialize before a class call ? What will you do if you want to use same instance of any dep? – Engineert Jul 10 '19 at 13:19
  • @VincentSavard I agree but I actually find one parameter easier to maintain in the long run – IMB Jul 10 '19 at 14:03
  • @Engineert Yes instantiating foosDependencies instantiates all. I don't understand your second question. – IMB Jul 10 '19 at 14:04
  • @IMB You have 3 dependencies 'dep1, dep2,dep3'. Assume dep1 is your dbContext and you need to use same instance with all classes call. How you can overcome this issue when you initialize every instance every time? – Engineert Jul 10 '19 at 18:57
1

The Service Locator Pattern is what you are describing. This pattern involves passing a single object to a new object. This new object gets all of it's dependencies from this single object.

Most times this is an anti pattern, however there is an exception for dependency injection containers, which are basically a "service locator." Generally you'll see service locators at high levels of the application, where an entire application instance is being configured.

Once you get deeper than this global level, service locators become an anti pattern, because of the complexity of creating the service locator object. This makes it difficult to isolate these lower level components for unit testing, and the dependencies for these objects become difficult to understand.

So, there are valid use cases for both, however I would recommend against using this "service locator" object much further down than configuring the whole application itself.

| improve this answer | |
  • Isn't service locators a general locator/container for all classes an application might ever need? My example requires a dependency only specific to that class. e.g., Foo() only accepts FooDependencies() and not something like GeneralServiceLocator(). Is it still called service locator even if FooDependencies() is only meant to be used by Foo()? – IMB Jul 13 '19 at 12:19
  • @IMB: even if the object is meant to be used by a single class, it would still be a service locator. The point is a service locator is an object that has the dependencies of other objects. The better question is, if you are considering a service locator, maybe it is time to refactor code so you can pass fewer dependencies, and consolidate behavior in more specialized classes. – Greg Burghardt Jul 13 '19 at 13:23
1

The characteristic you are looking for is called cohesion. The simple heuristic to check if the cohesion is high, remove one dependency and check if other operations (methods) can be still performed.

| improve this answer | |
0

As stated before, there's no reason to necessarily stop yourself at 3 dependencies. There's no hard and fast rule that dictates that classes with more than 3 dependencies are absolutely evil. Sure, a large number of dependencies can be a code smell, but that's not to say that there's never a valid reason to have them. It simply serves as a warning that you may need to start considering a slight refactor if a class is becoming too bloated.

Like Thomas stated in his answer, cohesion is one possible goal. And in the case of DI, you really should be striving for high cohesion and loose coupling. If there are dependencies that can be removed from a class and certain functionality is unaffected, that may be a sign that the specified functionality can be moved elsewhere. However, this is not always the case!

You may simply have certain Manager classes that simply require a larger than normal amount of dependencies, but the most important part is to just vet the code you're working on and apply the principals and "rules" responsibly where necessary.

| improve this answer | |
  • I agree with high cohesion but it's unclear to me if my question was answered. For the sake of argument, let's say a class can't be split any further and it really needs 10 dependencies. Using/refactoring the parameter positions of that class is going to be tedious. Given this case, is my proposed consolidation of all dependencies in a class acceptable or is it considered evil? The way I see it, the class is more maintainable than having to deal with 10 parameter positions. – IMB Jul 10 '19 at 18:10
  • As a personal approach, I would explicitly list my dependencies. If you're hiding them behind some other container, all you're doing is further obfuscating what's happening in a class. If a class NEEDS those 10 dependencies, then inject them. Sure it looks ugly, but a glance at the constructor will be a lot more informative than having to traverse to some other class simply to determine what dependencies the current class you're working in has. – JD Davis Jul 10 '19 at 18:19
  • It's not really about being being ugly, I'm just finding it harder to maintain. The container class is basically just a factory of dependencies. I personally find it cleaner and maintainable as it also explicitly list the dependencies. So I guess it's just a matter of personal preferences, nothing particularly smelly? – IMB Jul 10 '19 at 18:24
  • A large number of dependencies is always going to have some sort of smell, but I'd argue that throwing them into a container class (at least in this case), is only going to make the smell worse. I'm not going to say that there isn't at least some valid reason to have the wrapper, but I'd say that if needing to use wrappers becomes a norm, you may want to look at your development strategy that's causing those classes to become bloated in the first place. – JD Davis Jul 10 '19 at 18:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.