Where should new objects be created?

[NotionCommotion]

I found it helpful to configure all my objects in one or several bootstrap scripts.  Before doing so, I would often find myself forgetting that I already had written some class which is located deep in some script and recreating it for some other need (granted, composer definitely helped in this regard, but using it wasn't always applicable).

<?php

$c=new Pimple\Container(getMySettings());

$c['pdo'] = function ($c) {
    $db = $c['settings']['mysql'];
    return new \PDO(bla);
};

$c['foo'] = function ($c) {
    return new Foo(
        $c->get('pdo')
    );
};

$c['bar'] = function ($c) {
    return new Bar(
        $c->get('foo')
    );
};

It is all good until I find myself needing to create some new instance of some class instead of just passing some common instance to whoever needs it.

class Foo
{
    private $pdo;

    public function __construct(\PDO $pdo)
    {
        $this->pdo=$pdo;
    }

    public function getSomething():SomeClass
    {
        return new SomeClass($this->getAllDataFromDB($GET['id']));
    }
}

class SomeClass
{
    private $arr;

    public function __construct(array $records)
    {
        foreach($records as $record) {
            $arr[]=new SomeOtherClass($record);
        }
    }
}

I've read that one shouldn't create new objects in a given class's constructor, and while I could instead do so in some class's method and pass the object to the given class's constructor, to me it seems to really be the same thing.  The three issues I have with this scenario are:

1. Needing to remember that these classes exist as they are hidden deep in some classes.
2. Makes it more difficult to replace one of the classes with some other class in the future.
3. Some duplication of code.

Are there other compelling reasons not to do so?  Is it worth the effort to do differently?

If so, how would you recommend doing so?  My thoughts would either to use Pimple's factory() method are create my only factory which also relies on anonymous functions.  Maybe something else?

$c['someClass'] = $c->factory(function (array $record, Container $c) {
    return new SomeClass($record);
});

Thanks

[requinix]

Not everything has to go through dependency injection. It's okay to hardcode some class names. You can always refactor later.
If, on the other hand, you know right now that you want to inject those SomeOtherClass instances, pass the container to your constructor and configure the container to create them.

Regarding not creating things in a constructor, that's not quite right. What's not necessarily good is creating dependencies in the class. Which is sometimes a nebulous term - see also my first sentence.

[NotionCommotion]

20 hours ago, requinix said:

Not everything has to go through dependency injection. It's okay to hardcode some class names. You can always refactor later.
If, on the other hand, you know right now that you want to inject those SomeOtherClass instances, pass the container to your constructor and configure the container to create them.

Regarding not creating things in a constructor, that's not quite right. What's not necessarily good is creating dependencies in the class. Which is sometimes a nebulous term - see also my first sentence.

Fair enough.  Sometimes a little more art than science...

[kicken]

One way to help decide if you should bother with DI is to ask whether anyone outside of your code has an real reason to create said object.

As an example, I has a project where there was a service to start background jobs.  The method startJob() would return a Job object that detailed things about the newly created job and have a couple methods to manage it.  There's really no reason for anyone other than that method to try and construct that class so there's not much point in going though DI for it.  Instead it just does return new Job($details);

 

[JacobSeated]

 

On 12/6/2019 at 4:10 PM, NotionCommotion said:

 

1. Needing to remember that these classes exist as they are hidden deep in some classes.
2. Makes it more difficult to replace one of the classes with some other class in the future.
3. Some duplication of code.

Are there other compelling reasons not to do so?  Is it worth the effort to do differently?

If so, how would you recommend doing so?  My thoughts would either to use Pimple's factory() method are create my only factory which also relies on anonymous functions.  Maybe something else?

You will probably not get a complete answer on a forum, as this is a fairly large and complex topic. People are writing entire books on OOP best practices and design patterns, and many of us are still learning about these things while trying to develop and maintain existing projects.

I think you will find that Dependency Injection is hugely beneficial, even just on personal projects, at least this is my experience after I started using it. At first it is not very obvious, but it should become more obvious as you continue using it.

The thing is with OOP, it kinda forces you to consider the overall design of your application, and if you also follow DRY (Don't Repeat Yourself), then you will almost automatically force yourself into thinking about the design patterns.

Do not expect perfection from the start. It is hard avoid writing ineffective code while learning, but don't stop thinking about your solutions and best practices. You can always re-organize things as you learn. 

What I personally found, is that inheritance (extends) are bad, because it couples your code and makes it harder to re-use. Instead, I am now using DI almost exclusively. Chances are that you will regret using "extends" because of following DRY, and when later realizing that you could re-use that code somewhere else. If you want to re-use a class that extends another class, then you are forced to instantiate the whole object. Not only is that ineffective, it is also not always possible, since some of that code might connect to a database or API which you do not need..

Another thing that is bad, which you pointed out yourself, is instantiating dependencies (classes) inside the class that relies on them. I used to do this for a very long time, thinking it made it easy to instantiate the classes without having to worry about instantiating the dependencies from my composition root. This is true, to a certain extent. But, it also creates a hard-coupling which makes it impractical to maintain and re-use the code as your project grows—even within your own personal project. So, no usage of new inside other classes, unless you use it in a factory class 🙂

When your dependencies themselves have dependencies, it will be impractical to instantiate them inside the constructors of classes, since you will have to update every single class if one of the dependencies changes its dependencies. Finally, it also takes more mental energy and time to assemble an object if you need to run tests on it.

If you must have code duplication (usually a code-smell), then you can also consider using traits. A trait is basically a copy/paste of code that is shared by multiple classes.

Now we are just discussing the code aspect, but your file system structure is just as important in keeping things simple. Hope this helps, and please note I am no expert, I am still learning about all this myself. 

Edited December 12, 2019 by JacobSeated

[gizmola]

The main argument against using 'new' to create objects inside a class, is that it now becomes very difficult to unit test your code, you now have no way of mocking the dependent class.  If you use a service container, then you can mock the container itself, so long as you pass the DIC to the class that needs to create the objects internally.