[FR] Use Craft::createObject instead of "new" Object

[Anubarak]

Description

This is not really an issue for one single spot, but a general request for "coding style". I notice many times you create classes with the new keyword which makes it hard to fully use the potential of Yii2. Things like Exceptions and Queries don't need to be created via DI but especially components in your ComponentHelper could be created in that way to achieve Dependency Injection.

I use Yii2 outside of the Craft world too and since the upcoming release (Yii3) is going to (kinda) drop the Service Locator my modules for Yii3 are going to drop all of the Yii::$app-> lines as well. In order to prepare for that move tt would be awesome if Craft would let me do that easier by also using the createObject method to allow me to pass dependencies.

Of course if you are going to not use Yii3 in the future and use another Framework due to the heavy changes or if you are going to stay with Yii2 for the next years it won't make any differences for you,

PS I'm talking about this

[brandonkelly]

We were just talking about how we need to start refactoring code in preparation for that, whether we go with Yii 3 or Laravel, etc., it seems DI is the way to go these days.

[Anubarak]

Yeah I really don't want to make that decision. Of course you could bring back the Service Locator in Yii3 (only a one liner) but to choose between the ongoing trend/"best practice" and easy to use code can be quite difficult.
I guess your internal discussion is going to be very interesting.

In fact Craft can be moved to Yii3 even with all those upcoming breaking changes without that much effort to restructure logic. I searched all of your components and you avoided dependency cycles quite well but for example the Commerce package won't be that easy to refactor without a service locator because some classes would look like

class Customers{
    public function __construct(Carts $carts){}
}
//... 
class Carts{
    public function __construct(Customers $customers){}
}
// ...
$customers = Commerce::getInstance()->getCarts();
// fatal error, out of memory, Carts tries to create a Customers instance, 
// Customers tries to create a Carts instance and so on

Events can be migrated as well - it's just much more effort in my opinion. Behaviors such as FieldLayoutBehavior can either be integrated via trait or a 3rd party library could be used. The console will be Symfony so that won't be an issue and since you didn't work that much with ActiveRecords except storing Data moving to (for example) Cycle won't be such a huge issue.
When I thing about it again, after writing this, Yii3 does not feel like Yii anymore but like a composer.json for many different packages.

Just my 2 cents, I know this was much offtopic so I could delete it if you don't like it

[brandonkelly]

Appreciate the pointers! The main reason we had brought it up recently was to make the code more testable, with the added benefit that it will help update to Yii 3 or switch frameworks down the road.

[Anubarak]

@brandonkelly I know this is a bit late but please would you use Craft::createObject() for Craft 3.5 in your new FieldLayout settings?
I can even do a pull request for it and replace everything. I would really like to extend the class an add custom properties to it in order to store additional attributes in your elements.

I would love to extend your Customfield and change the settingsHtml function and that's totally possible when you create the class via DI-Container.
I'll create a pull request if you allow it

All you need to do is to change

$element = new $type();
Craft::configure($element, $config);

to

/** @var FieldLayoutElementInterface $element */
$config['class'] = $type;
$element = Craft::createObject($config);

Here a working example with only 3 changes in your source Code and this custom class

namespace modules\fieldlayoutelements;

use Craft;

class CustomField extends \craft\fieldlayoutelements\CustomField
{
    public $sortable = 'JA';

    public function settingsHtml()
    {
        $view = Craft::$app->getView();
        $settingsHtml = parent::settingsHtml();
        $settingsHtml .=  $view->renderTemplateMacro('_includes/forms', 'textField', [
            [
                'label' => Craft::t('app', 'Is this field sortable via frontend?'),
                'id' => 'sortable',
                'name' => 'sortable',
                'value' => $this->sortable,
                'placeholder' => 'sortierbar',
            ],
        ]) ;
        return $settingsHtml;
    }
}

[brandonkelly]

@Anubarak Done!

[Anubarak]

@brandonkelly Thank you so much. I added 2/3 other parts as well and added those in the changelog via pull request.
You have no idea how much this is going to improve our experience with Craft.
The new field layout designer is so great and we all love it.
It going to be nearly perfect with the ability to add custom properties to it.
(Sorry for all the extra work)