anttwuan
anttwuan

Reputation: 101

PHP Class Dependency Injection Failure

Okay, I've often had some unsolved errors when it comes to accessing several other classes in one class. Depending on which class gets required first, it will not make it accessible in the __construct function in the other files.

Undefined property: Container::$Core in /usr/share/nginx/html/app/classes/class.users.php on line 9


class.container.php

public function __construct() {
    if(isset(self::$class)) {
        foreach(self::$class as $key => $value) {
            if(!isset($this->{$key})) {
                $this->{$key} = $value;
            }
        }
    }   
} 

public function setClassHandlers($class = []) {
            if(!isset(self::$class)) {
                foreach($class as $name => $file) {
                    $file = DIR . "app/classes/" . $file;

                    if(!isset(self::$library[$name])) {
                        // Get interface file
                        $interface = DIR . "app/classes/interfaces/interface.".strtolower($name).".php";
                        // Check if interface file exists
                        if(!file_exists($interface)) {
                            // Throw exception
                            $this->throwException("Unable to load interface file: {$interface}");
                        }

                        // Require interface
                        require_once $interface;
                        //Check if interface is set
                        if(!interface_exists("i".ucfirst($name))) {
                            // Throw exception
                            $this->throwException("Unable to find interface: {$interface}");
                        }

                        // Check if class file exists
                        if(!file_exists($file)) {
                            // Throw exception
                            $this->throwException("Unable to load class file: {$file}");
                        }
                        // Require class
                        require_once $file;
                        // Check if class file exists
                        if(class_exists($name)) {
                            // Set Library
                            self::$library[$name] = $file;
                            // Set interface
                            self::$interface[$name] = $interface;
                            // Set class        // class.container.php
                            self::$class[$name] = new $name(new self);
                            $this->{$name} = self::$class[$name];
                        } else {
                            // Thror error
                            $this->throwException("Unable to load class: {$name}", self::$library[$name]);
                        }

                    }
                }
            } 
        }

Inserting arguments into the function on index.php:

require_once DIR . 'app/management/dependency.php';

$container = new Container();

$container->setClassHandlers([
    // Name         // Path
    'Database'  => 'class.database.php',
    'Config'    => 'class.config.php',
    'Users'     => 'class.users.php',
    'Core'      => 'class.core.php',
    //'Admin'       => 'class.admin.php',
    //'Forum'       => 'class.forum.php',
    //'Template'    => 'class.template.php'
]);

class.users.php

public function __construct(Container $class) {
        $this->db   = $class->Database;
        $this->core = $class->Core;
        $this->ip   = $this->core->userIP();
        $this->container = $class;
    }

For example both Users and Core are used within the same file of eachother, but as mentioned above, if Core gets required first, Users is not an available dependency inside that class.
I'm not really quite sure how to fix this, so every help is appreciated.

Upvotes: 2

Views: 435

Answers (1)

yivi
yivi

Reputation: 47380

Your "container" is trying to instantiate the contained objects when the setClassHandlers method is run, and by that time not all its properties might be set.

Also, your container is not "lazy" enough. is trying to instantiate everything right now, even if it might not be needed.

Try the following

First, remove these lines:

    // Set class        // class.container.php
    self::$class[$name] = new $name(new self);
    $this->{$name} = self::$class[$name];

Then, add a private array services to your Container class:

private $services = [];

And finally, add a magic getter to your container:

function __get($service)
    {
        if ( ! isset($this->services[$service]) ) {
            $this->services[$service] = new $service();
        }

        return $this->services[$service];
    }

Again, learning about this is great, but I would advise you take a look at some other implementations to learn from them. Pimple's is great, very simple, and easy to follow.

Here we are injecting the container on all your objects instead of the object concrete dependencies, which is generally (an rightly) frowned up upon. But I don't want to introduce more modifications to your design, is better that you learn those by yourself.

On top of that, your container is also handling what would be better handled by an autoloader, and mixing responsibilities.

Also, as I said in the comments, you are reinventing the __DIR__ constant, which already exists in PHP. You are using

define( 'DIR', dirname(__FILE__) . '/' );

which is pretty much equivalent to __DIR__ (Or, more precisely, to __DIR__ . '/').


Finally, most of your troubles are caused by circular dependencies, which your container (nor any container) can't fix. A depends on B which depends on A. The only way you could get around that is by using a setter to inject your dependencies.

A simpler implementation, more or less following your code:

For autoloading:

function autoload_so($class)
{
    $file = __DIR__ . "/app/classes/class." . strtolower($class) . '.php';
    if (file_exists($file)) {
        include_once($file);

        return true;
    }

    return false;
}

spl_autoload_register('autoload_so');

I'm not using namespaces, and I disregarded your interface logic. Which was a bit weird as well. You'll need to implement your own loading for interfaces, you should be able to do so.

For the container:

class MyContainer
{
    public $services = [];

    function __get($service)
    {
        if ( ! isset($this->services[$service]) ) {
            $this->services[$service] = call_user_func([$this, "get_$service"]);
        }

        return $this->services[$service];
    }

    /**
     * @return Users
     */
    private function get_Users()
    {
        return new Users($this->Database, $this->Core);
    }

    /**
     * @return Core
     */
    private function get_Core()
    {
        return new Core();
    }

    /**
     * @return Database
     */
    private function get_Database()
    {
        return new Database($this->Core);
    }

}

Notice that

  • You'll need a new method for each new "service" you want to add.
  • I'm injecting the dependencies directly in the constructor

Finally your classes would be something like:

Database

<?php

class Database
{
    public function __construct(Core $core)
    {
        $this->core  = $core;
    }
}

Users

class Users
{
    public $ip;

    public function __construct(Database $db, Core $core)
    {
        $this->db   = $db;
        $this->core = $core;
        $this->ip   = $this->core->userIP();
    }

}

etc.

Again, this is all very crude. I would use namespaces and a more general autoloader and directory structure.

All this should be enough for a starting point for you to build your own.

But no container will magically fix your circular dependencies for you.

Upvotes: 1

Related Questions