Cyclic Dependency Injection and Making a Choice

Cyclic dependency injection is your code asking you to make a choice rather than remain on the fence.

Here’s a story of what happened in my case. I ran across a troubling case of cyclic dependency injection in the constructors of some code. Here is the code for two simple classes where a CustomerModel checks if it can do something.

class CustomerModel extends UserModel {
    private $_acl;
    public function __construct(AclModel $acl) {
        $this->_acl = $acl;
    }
    public function foo() {
        if ($this->_acl->isAllowedToDoFoo()) {
            // do foo things
            return true;
        } else {
            return false;
        }
    }
    public function getFooLevel() {
        return $this->_user['level_of_foo'];
    }
}
class AclModel {
    private $_customer;
    private $_security;
    public function __construct(CustomerModel $customer) {
        $this->_customer = $customer;
        $this->_security = true; // default
    }
    public function isAllowedToDoFoo() {
        if (!$this->_security || $this->_customer->getFooLevel() > 10) {
            return true;
        } else {
            return false;
        }
    }
    public function setSecurity($bool) {
        $this->_security = $bool;
    }
}

The idea seems good. The access control logic will exist separate from the customer logic.

In this circular dependency injection, CustomerModel depends on AclModel to call isAllowedToDoFoo() and likewise, AclModel depends on CustomerModel to call getFooLevel(). This means that the constructor will produce an infinite stack.
Example of infinite constructor:

$customer = new CustomerModel(new AclModel(new ????));

So, according to Miško Hevery there is a solution to circular dependency. You can expose a hidden 3rd class that takes both the AclModel and the CustomerModel and decouples the connected methods.

Initially, I thought that the scope of responsibility of the CustomerModel should be restricted to dealing with the customers, and not dealing with any kind of checking. In which case, the access control logic moves outside of CustomerModel, say, into a controller or a super model such as ProtectedCustomerModel. Of course, this means that things such as access control and data validation gets pushed out of the model, which makes it a bit less self-contained. CustomerModel itself would no longer need an AclModel, and ProtectedCustomerModel would need both an AclModel and a CustomerModel.

class ProtectedCustomerModel {
    private $_acl;
    private $_customer;
    public function __construct(AclModel $acl, CustomerModel $customer) {
        $this->_acl = $acl;
        $this->_customer = $customer;
    }
    
    public function foo() {
        // notice how this is a merging of AclModel and CustomerModel
        if (!$this->_acl->getSecurity() || $this->_customer->getFooLevel() > 10) {
            $this->_customer->foo();
            return true;
        } else {
            return false;
        }
    }
}
class CustomerModel extends UserModel {
    public function __construct() {
        // no more dependency on AclModel
    }
    public function foo() {
        // do foo things
    }
}
class AclModel  {
    private $_security;
    public function __construct() {
        // no more dependency on CustomerModel
    }
    public function setSecurity($bool) {
        $this->_security = $bool;
    }
    public function getSecurity($bool) {
        return $this->_security;
    }
}

But one of the interesting consequences of this solution is that the AclModel isn’t actively performing the access control. It’s been moved to ProtectedCustomerModel, and so the rest of the AclModel code could potentially move there.

class ProtectedCustomerModel {
    private $_customer;
    private $_security;
    public function __construct(CustomerModel $customer) {
        $this->_customer = $customer;
        $this->_security = true; // default;
    }
    public function foo() {
        // notice how this is a merging of AclModel and CustomerModel
        if (!$this->_security || $this->_customer->getFooLevel() > 10) {
            $this->_customer->foo();
            return true;
        } else {
            return false;
        }
    }
    public function setSecurity($bool) {
        $this->_security = $bool;
    }
}
class CustomerModel extends UserModel {
    public function __construct() {
        // no more dependency on AclModel
    }
    public function foo() {
        // do foo things
    }
}

In which case, all we really did was move access control checking to AclModel and renamed AclModel into ProtectedCustomerModel. We can’t really keep the name AclModel because it doesn’t make sense to have AclModel::foo() which is an operation, rather than an access control check.

We could also use inheritance:

class ProtectedCustomerModel extends CustomerModel {
    private $_security;
    public function __construct() {
        $this->_security = true; // default;
    }
    public function foo() {
        // notice how this is a merging of AclModel and CustomerModel
        if (!$this->_security || $this->_user['level_of_foo'] > 10) {
            parent::foo();
            return true;
        } else {
            return false;
        }
    }
    public function setSecurity($bool) {
        $this->_security = $bool;
    }
}
class CustomerModel extends UserModel {
    public function __construct() {
        // no more dependency on AclModel
    }
    public function foo() {
        // do foo things
    }
}

You can see, we don’t even need getFooLevel() or getSecurity() anymore.

Quite equivalently, the access control logic could have completely been merged into CustomerModel. And now, we don’t have an AclModel nor a ProtectedCustomerModel, and we only need 1 class.

class CustomerModel extends UserModel {
    private $_security;
    public function __construct() {
        $this->_security = true; // default
    }
    public function foo() {
        if (!$this->_security || $this->_user['level_of_foo'] > 10) {
            // do foo things
            return true;
        } else {
            return false;
        }
    }
    public function setSecurity($bool) {
        $this->_security = $bool;
    }
    public function getFooLevel() {
        return $this->_user['level_of_foo'];
    }
}

But the downside is that we’ve lost some amount of separation of concerns. This doesn’t have the feel of object-oriented code. But that really boils down to the key design question: do you want CustomerModel to encapsulate access control, or do you want to separate it out into a ProtectedCustomerModel class?

It cannot be separate, yet encapsulated, at the same time. An attempt to do both at the same time creates the problematic cyclic dependency injection. You must decide, separate OR not separate.

You must weigh the consequences of doing things either way. You might end up with too many small classes. You want to avoid giant classes. There is balance in trying to make the code understandable. There is no right or wrong answer, only an answer that shifts the priority to have one thing, at the cost of another.

Of course, you could also use Setter Injection, but that doesn’t eliminate the cycle and it is really a cop-out to avoid making a decision, and you put your code at risk of being ambiguous.

About Tim

has written 28 post in this blog.

4 Responses to "Cyclic Dependency Injection and Making a Choice"

  • M Coe says:
  • Tim says:
  • Jan Judas says:
  • Tim says:
Leave a Comment