Dependency injection on custom classes stuck in indefinite loop in L4.2?

Question

I've been changing my Controllers and helper classes to use dependency injection, and it seems I've got my helper classes stuck in an infinite loop.

Below is my custom ServiceProvider and the two sample helper classes. As you can see, they inject each other, so they keep going back and forth.

What's the solution to this issue? What mistake do I seem to be making? What can I do so that I can run tests on helper classes like General and Person, while mocking the helper classes that are called from inside of them?

One way that I guess could work is in my ServiceProvider, do the following:

if (isset($appmade->General)) { 
    // inject the General app that's already instantiated 
} else { 
    $abc = app::make('\Lib\MyOrg\General'); 
    $appmade->General = $abc; 
}

Is that the correct way?

// /app/providers/myorg/MyOrgServiceProvider.php

namespace MyOrg\ServiceProvider;
use Illuminate\Support\ServiceProvider;
class MyOrgServiceProvider extends ServiceProvider
{
    public function register()
    {
        $this->app->bind('\Lib\MyOrg\General', function ($app) {
            return new \Lib\MyOrg\General(
                $app->make('\Lib\MyOrg\Person'),
                $app->make('\App\Models\User')
            );
        });

        $this->app->bind('\Lib\MyOrg\Person', function ($app) {
            return new \Lib\MyOrg\Person(
                $app->make('\Lib\MyOrg\General'),
                $app->make('\App\Models\Device')
            );
        });
    }
}

// /app/libraries/myorg/general.php

namespace Lib\MyOrg;
use App\Models\User;
use Lib\MyOrg\Person;
class General
{
    protected $model;
    protected $class;

    public function __construct(Person $personclass, User $user) 
    {
    }
}

// /app/libraries/myorg/person.php

namespace Lib\MyOrg;
use App\Models\Device;
use Lib\MyOrg\General;
class Person
{
    protected $model;
    protected $class;

    public function __construct(General $generalclass, Device $device) 
    {
    }
}

@hek2mgl Are you referring to the way the code is displayed in the question? Looks ok to me...please submit an edit if you have readability suggestions — Luke Shaheen, Oct 24 '14 at 18:13
@John This is not good. Not good at all. You need to refactor your classes. Avoid circular dependency. — brainless, Oct 28 '14 at 07:46
@John Please provide the scenario why do you need circular dependency. That will understand the core of the problem and to arrive at a solution! — brainless, Oct 28 '14 at 07:47
@brainless I don't need circular dependency :) I realize that's what I'm ending up with here, so the question is how can I REMOVE the circular dependency from this situation. — Luke Shaheen, Oct 28 '14 at 12:36
@John To figure out how to remove the circular dependency, see the "rule of thumb" at the end of my answer for a general approach. Otherwise, for more specific help, we would have to know the contents of your 2 helper classes, and what you're trying to accomplish with each of them, in order to parse out what could be extracted to a 3rd class. — damiani, Oct 28 '14 at 13:08
@damiani Your answer makes perfect sense, and sort of made me slap myself and say duh, thanks! :) I just haven't had a chance to get to executing yet, plus I figured spending +200 rep was worth keeping the question open until the end :p — Luke Shaheen, Oct 28 '14 at 13:25
@John I hear ya...All I'm saying is that in order to get any more *specific* ideas on how to untangle the dependencies, you would need to post more info about your classes. — damiani, Oct 28 '14 at 13:34

score 13 · Answer 1 · answered Oct 25 '14 at 03:46

Your helper classes have a case of circular dependency which, when mixed with dependency injection, make them un-instantiable...and there's no good way to get them to work and be easily testable.

There are some hack-y ways to get this to work:

You could use setter injection, injecting the dependency in a setter method when the helper is used, rather than in the constructor when it is instantiated. This has many disadvantages, poor testability being chief among them. (See http://symfony.com/doc/current/components/dependency_injection/types.html)
In Laravel 5, you could use method injection, particularly if the dependency is only pertinent to one or two methods. (See http://mattstauffer.co/blog/laravel-5.0-method-injection)
You could do an existence check like you propose above, or something with an event listener. But all of this is masking the fact that...

A circular dependency usually indicates that a design change is in order, and is best solved by reconsidering how your classes interact with each other.

If your Person and General classes depend fully on each other, then it's possible they share a single responsibility and should be combined into a single class.
If, however, they rely on a subset of each other's functionality, then there's probably a separate class with its own responsibility hiding in there somewhere, and then the best solution would be extract the common functionality into a 3rd class. That way, rather than having Class A rely on Class B and Class B rely on Class A, both Classes A and B would instead rely on Class C. You can safely instantiate them all, and they all remain easily testable.

How can you easily figure out what your new Class C should contain? A great rule of thumb comes from http://misko.hevery.com/2008/08/01/circular-dependency-in-constructors-and-dependency-injection/... "List all of the methods in your class A used by class B, and all the methods in your class B used by class A. The shorter of the two lists is your hidden Class C."

Dependency injection on custom classes stuck in indefinite loop in L4.2?

1 Answers1