Skip to content

Latest commit

 

History

History
183 lines (136 loc) · 5.6 KB

ObjectInstantiationSniff.md

File metadata and controls

183 lines (136 loc) · 5.6 KB

Rule: Keyword new should not be used

Background

The new keyword should not be used to instantiate new classes. Constructor DI should be used instead: Inject a factory (e.g. ProductInterfaceFactory) which can be generated automatically, Magento then takes care of which actual class to instantiate.

This is an experimental rule, only applied with lowest severity treshold of 1.

Reasoning

Magento uses constructor dependency injection with autowiring. It automatically generates interceptors that extend the original class to add plugins.

It is encouraged to use these mechanisms instead of instantiating objects directly with new. This way, Magento can transparently replace the concrete class with an interceptor or alternative implementations based on DI preference configuration.

This flexibility with plugins and preferences is lost if classes are instantiated directly.

Exceptions

  • Instantiating exceptions with new is allowed, there is no need for alternative implementations or plugins in exceptions
  • The rule does not apply to unit tests, where automatic constructor DI is not present

Dispute

Classes from PHP core or non-Magento libraries: While it is possible to generate factories for them too, it is considered overkill by many.

Compare

new DateTime('yesterday')

vs

$this->dateTimeFactory->create(['time' => 'yesterday'])

Data transport objects: Data transport objects (DTOs) are often used in events to allow changing values in observers:

new \Magento\Framework\DataObject(['key' => 'value])

Using factories seems unnecessary too here, and Magento does not do it in the core either.

As an alternative, since DataObject is not more than a glorified array with magic getters and setters, an stdclass object could be used instead, circumventing the new rule: (object)['key' => 'value']

Anonymous classes: For anonymous classes there is no alternative without new and they might be useful as "private classes", i.e. internal to a module, where plugins are not useful or desired.

How it works

Any usage of new that is not preceded by throw is considered a violation

How to fix

Given code like this:

namespace Lotr;

use Lotr\Ring;

class Sauron
{
    public function forgeTheOneRing()
    {
        $ring = new Ring('Ash nazg durbatulûk, ash nazg gimbatul, ash nazg thrakatulûk, agh burzum-ishi krimpatul');
        $this->putOnFinger($ring);
    }
}

you would introduce constructor dependency injection with an autogenerated factory like this:

namespace Lotr;

use Lotr\RingFactory;

class Sauron
{
    /**
     * @var RingFactory
     */
    private $ringFactory;

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

    public function forgeTheOneRing()
    {
        $ring = $this->ringFactory->create(
            [
                'inscription' => 'Ash nazg durbatulûk, ash nazg gimbatul, ash nazg thrakatulûk, agh burzum-ishi krimpatul'
            ]
        );
        $this->putOnFinger($ring);
    }
}

Note that the argument for create is always an array where the keys have to equal the constructor argument names for the class to be instantiated. So the example above assumes a constructor signature for Ring like:

public function __construct(string $inscription);

If there is an interface for Ring, e.g. RingInterface it is recommended to use a generated factory for the interface, so that the actual implementation may be exchanged:

public function __construct(RingInterfaceFactory $ringFactory)

Special case: Custom factory

Sometimes, the automatically generated factories do not quite serve our needs and more specific creation methods than the generic create are helpful. You can create your own factory implementation, Magento will only generate a factory class if it cannot find an existing one.

Let's say, we want specific factory methods for different rings, we could implement our own factory class like this:

namespace Lotr;

class RingFactory
{
    public function createElvenRing() { ... }
    public function createDwarfRing() { ... }
    public function createHumanRing() { ... }
    public function createTheOneRing() { ... }
}

In this factory, we still won't use the new keyword, but instead the object manager can be used (factories are an exception to the rule "Never use the object manager directly"!)

namespace Lotr;

use Lotr\Ring;

class RingFactory
{
    /**
     * @var \Magento\Framework\ObjectManagerInterface
     */
    private $objectManager;

    public function __construct(\Magento\Framework\ObjectManagerInterface $objectManager)
    {
        $this->objectManager = $objectManager;
    }

    public function createTheOneRing(): Ring
    {
        return $this->objectManager->create(
            Ring::class,
            [
                'inscription' => 'Ash nazg durbatulûk, ash nazg gimbatul, ash nazg thrakatulûk, agh burzum-ishi krimpatul'
            ]
        );
    }
}

Do not use the object manager like this in anything else than factory or builder classes! In other words, object creation and object usage must be separated.

The signature of ObjectManager::create() is similar to the create() method of the automatically generated factories, but additionally needs a class or interface name as first argument.

With such a custom factory, Sauron can forge the ring and let the "ring factory" decide about the inscription:

    public function forgeTheOneRing()
    {
        $ring = $this->ringFactory->createTheOneRing();
        $this->putOnFinger($ring);
    }