Containerization of services

[webnitros]

What does it do?

added the ability to call containers from any part of the code without the need to connect an instance with modx

Why is it needed?

Describe the issue you are solving.

# example №1
echo \MODX\Revolution\Services\Container::getInstance()->get('modx')->getOption('site_name');

# example №2
class App
{
    public function site_name()
    {
        $custom = \MODX\Revolution\Services\Container::getInstance()->get('modx')->getOption('site_name');
        return  mb_strtoupper($custom);
    }

    public static function service()
    {
        return \MODX\Revolution\Services\Container::getInstance()->get('app');
    }
}

\MODX\Revolution\Services\Container::getInstance()->add('app', new App());

$App = \MODX\Revolution\Services\Container::getInstance()->get('app')->site_name();

echo App::service()->site_name();

The main feature is the full use of the composer without the need to additionally pass an instance of $modx to the class

public function __construct(modX $modx = null)
{
      $this->modx = $modx;   
}

In the future, you can simplify the call to modx through the function

if (!function_exists('modxApp')) {
    function modxApp()
    {
        return \MODX\Revolution\Services\Container::getInstance()->get('modx');
    }
}

Works similarly in Laravel
https://packagist.org/packages/illuminate/container
does not cause any problems

the code was taken from illuminate/container,
https://github.com/illuminate/container/blob/a0796471eff9649592d7b5bdc8f705c134ceaaac/Container.php#L1407

is a public singleton pattern

 /**
     * The current globally available container (if any).
     *
     * @var static
     */
    protected static $instance;

    /**
     * Get the globally available instance of the container.
     *
     * @return static
     */
    public static function getInstance()
    {
        if (is_null(static::$instance)) {
            static::$instance = new static();
        }
        return static::$instance;
    }

    /**
     * Set the shared instance of the container.
     *
     * @param Container|null $container
     * @return static
     */
    public static function setInstance(ContainerInterface $container = null)
    {
        return static::$instance = $container;
    }

How to test

Tested the web interface and with using the code above
phpunit tests via Github Actions completed successfully

Related issue(s)/PR(s)

Have not found

[sergant210]

Не должно быть никакого setInstance(). Это нарушение шаблона Singleton.

[webnitros]

Не должно быть никакого setInstance(). Это нарушение шаблона Singleton.

Я сперва так и сделал

$container = Container::getInstance();

Но phpunit тесты сделаны видимо не совсем правильно
и они валиться начинают (webnitros@7197b5d)

Возможно я не совсем верно упомянул о singalton, все таки это абстракция
Но контенеризация в Laravel так и работает https://github.com/illuminate/container/blob/a0796471eff9649592d7b5bdc8f705c134ceaaac/Container.php#L1422

[opengeek]

modX is the app and you would never want this in the container.

[opengeek]

I appreciate the contribution, but I do not see the need for this. I am not a fan of Laravel-style static helper methods like this, and you can get the container from the modX app instance.

[webnitros]

I appreciate the contribution, but I do not see the need for this. I am not a fan of Laravel-style static helper methods like this, and you can get the container from the modX app instance.

The problem I encountered is that every time I create a new class, I need to pass an instance of $modx to the constructor. This is inconvenient and leads to code duplication because I have to repeat this process for each class.

One way to solve this problem is by using a dependency injection container. The dependency injection container allows you to create and store object instances and provide them when needed.

Here's a simple example of how you can use a dependency injection container to solve the problem:

class modX
{

    public function getOption($name)
    {
        return 'http://site.com';
    }

}


class Container
{
    private static $instances = [];

    public static function getInstance($className, $modx = null)
    {
        if (!isset(self::$instances[$className])) {
            self::$instances[$className] = new $className($modx);
        }

        return self::$instances[$className];
    }
}

class MyClass
{
    private $modx;

    public function __construct($modx)
    {
        $this->modx = $modx;
    }

    public function doSomething()
    {
        return $this->modx->getOption('site_url');
    }
}

// Example usage:
$modxInstance = new modX(); // Creating an instance of $modx

$myClass = Container::getInstance('MyClass', $modxInstance);
echo $myClass->doSomething() . PHP_EOL;

// Later usage without modx instance
$myClass = Container::getInstance('MyClass');
echo $myClass->doSomething() . PHP_EOL;

Now you can create just one instance of $modx and use it in any class whenever necessary, without the need to pass it to the constructor every time.

To call a class without passing an instance of $modx to the constructor, you can use a static method, as in your example:

class App {
    public function getMsg()
    {
        return modX::getInstance('modX')::getOption('site_name');
    }
}

This getInstance method allows you to get a single instance of the modX class, which can be used anywhere in your application.

Currently, it works like this, for example, I created 10 classes:

class App {
    public function __construct(modx $modx)
    {
        $this->modx = $modx;
    }
    ....
}
class App2 {
    public function __construct(modx $modx)
    {
        $this->modx = $modx;
    }
    ....
}
class App3 {
    public function __construct(modx $modx)
    {
        $this->modx = $modx;
    }
    ....
}
class App4 {
    public function __construct(modx $modx)
    {
        $this->modx = $modx;
    }
    ....
}

I don't see the point of duplicating this code, but at the moment, it's necessary. Wherever we create a new class, we must always include __construct(modx $modx) in each class instance:

class App {
    public function __construct(modx $modx)
    {
        $this->modx = $modx;
    }
   public function app2()
    {
       # Or return new AppCustom($this)
        return new AppCustom($modx)
    }
}
class AppCustom {
   # or public function __construct(App $app)
    public function __construct(modx $modx)
    {
       # or $this->app = $app;
        $this->modx = $modx;
    }
   public function title()
    {
        # or return $this->app->modx->getOption('site_name')
        return $this->modx->getOption('site_name')
    }
}

Considering full-fledged development through PSR-&, this is clearly pointless code duplication. When the whole world is already using containerization.

[webnitros]

I appreciate the contribution, but I do not see the need for this. I am not a fan of Laravel-style static helper methods like this, and you can get the container from the modX app instance.

Currently, there is no method in modx to call like this: modY::getInstance('modY').

Although you can achieve this through:

    # custom modx.php class mody.php
    
class mody extends modX
    public static function getInstance($id = null, $config = null, $forceNew = false)
    {
        # this
        $class = __CLASS__; 
        # is replaced with this
        $class = class_exists($id) ? $id : __CLASS__;
        .................
        
        }

Naturally, this approach will require modification of index.php. $modx = modY::getInstance('modY');

modY::getInstance('modY')->getOption('site_name')

It works excellently, even with highly loaded projects, without causing any issues anywhere.

following the example from @sergant210 , implemented this method in many projects and followed it for about two years, observing the problems that modY::getInstance('modY') can cause.

Have not found

It is clear that no one will change index.php
Therefore, now there is a completely excellent opportunity to solve this issue through containerization, which is available in modx 3

Perhaps I didn't notice something, please give at least a code example when containerization for which the pull request was created will cause a problem? Tests passed.

[webnitros]

In my opinion, it is obvious that simplifying access to the application improves productivity. There is currently no reason not to accept this pull request. This code will not harm a simple user, but for professional development there will clearly be an increase.

[opengeek]

Modifying the index.php to call modX::getInstance() would solve the only problem you are trying to solve here, and I do not see any negative impact by changing the existing calls to the constructor to getInstance for this purpose. I would happily consider merging a PR that introduces this improvement. Creating redundantly nested instances of the app and container is not going to happen, though.

All this said, using modX::getInstance() inside classes that depend on the modX application is not proper design.