Broken Dependency injection by not using ObjectManager to get new objects

[mgrundkoetter]

You use \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance() quite often in the whole extension. This is only correct for creating the object manager itself as the call will NOT respect all the dependency injection stuff at all. So instead of doing things like:

$this->configurationManager = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Extbase\\Configuration\\ConfigurationManager');

this would be correct:

$this->configurationManager = $this->objectManager->get('TYPO3\\CMS\\Extbase\\Configuration\\ConfigurationManager');

or even better this:

/**
 * @var \TYPO3\CMS\Extbase\Configuration\ConfigurationManager
 * @inject 
 */
protected $configurationManager = null;

Also calls like

$news = new \Socialstream\SocialStream\Domain\Model\News();
$cat = new \GeorgRinger\News\Domain\Model\Category();
$classname = "\\Socialstream\\SocialStream\\Utility\\Token\\".ucfirst($type)."Utility";
return new $classname($pid);

do neither respect registered xclasses nor will any dependency injection work! All those calls should be made to ObjectManager::get() instead.

[kaystrobach]

@mgrundkoetter please do not use the @inject annotation ...

https://twitter.com/NamelessCoder/status/935808807510462465

Did you know that public property plus inject annotation still equals property gets set via Reflection? As in: even if you make it public, Extbase will not assign it as such. See https://t.co/E29KPXt6ej. Unfortunately, https://t.co/kBdnxX05ae is still true.

— Claus Due (@NamelessCoder) 29. November 2017

[mgrundkoetter]

@kaystrobach interesting point. As a lot of extensions (like news) still rely on Extbase DI, the object manager should be used nevertheless to not break this "feature" (and others) in other classes. So using GeneralUtility::makeInstance() is still not correct and should be replaced by ObjectManager::get() for anything else than the ObjectManager itself.

[kaystrobach]

@mgrundkoetter agree

anyway it's time to avoid the annotation and esp. the reflection stuff, as this has several speed implications.

[NamelessCoder]

Just as a side note, the "public still gets reflection-set" is no longer true, at least in master branch of TYPO3. It's still equally not recommended to make a public property with @inject since you're just moving the rule-breaking problem to a too-visible problem.

But whatever you do, you definitely want to use ObjectManager to make your instances.