Voitcus
Voitcus

Reputation: 4446

Why not "instantiate a new object inside object constructor"?

I answered a question (link) that I used a creation of the new object in another class' constructor, here the example:

class Person {
  public $mother_language;

  function __construct(){ // just to initialize $mother_language
    $this->mother_language = new Language('English');
}

And I got the comment from user "Matija" (his profile) and he wrote: You should never instantiate a new object inside object consturctor, dependencies should be pushed from outside, so anyone who uses this class knows what is this class dependent on!

Generally, I can agree with this, and I understand his point of view.

However, I used to do this this way very often, for example:

Please explain me, is it really important to avoid creating objects in constructors?

I've read this discussion but have still unclear mind.

Upvotes: 21

Views: 12649

Answers (7)

still_dreaming_1
still_dreaming_1

Reputation: 9135

There is nothing wrong with constructing new objects inside a constructor. There are advantages to both approaches. Taking in all constructor dependencies through the constructor arguments does make your class more flexible, and this flexibility especially comes in handy when writing automated tests for your class. But that also creates a layer of indirection. It also makes the class less convenient to use in the average use case because there are more constructor parameters to think about. It also makes it harder to know exactly how the class will behave because you don't know exactly what will be passed in. It makes the class less simple.

Taking in all the constructor dependencies through parameters is one way of using something called "inversion of control". I agree it is important to be familiar with this technique and to be capable of using it, but promoting is as fundamentally superior is exactly backwards. Inversion of control is evil, but sometimes it is a necessary evil. Some people would say it is always necessary. I don't agree with that, but even if I did, I would say this means it is always a necessary evil. Inversion of control makes your code less straightforward in the ways described above. When people argue in favor of inversion of control, what they are really saying is, "Using code is bad". I'm not saying people are intentionally saying that, because if they did they might not promote it. I'm saying there are non-obvious ways in which those concepts are saying the same thing, and hopefully by realizing they are saying this, some people will rethink it. I outline the reasons for this claim in my article "Using code is bad".

Upvotes: 0

ryanwinchester
ryanwinchester

Reputation: 12127

If you want to set a default, but improve flexibility/testability, why not do it like this?

class Person
{
    protected $mother_language;

    function __construct(Language $language = null)
    {
         $this->mother_language = $language ?: new Language('English');
    }
}

Some people still may not like it, but it is an improvement.

Upvotes: 2

Schleis
Schleis

Reputation: 43700

Yes it really is. You are then clear about what the object needs in order to be constructed. A large number of dependent objects being passed in is a code smell that perhaps your class is doing too much and should be broken in up in multiple smaller classes.

The main advantage of passing in dependent objects comes if you want to test your code. In your example, I cannot use a fake Language class. I have to use the actual class to test Person. I now cannot control how Language behaves to make sure that Person works correctly.

This post helps explain why this is a bad thing and the potential problems that it causes. http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-real-work/

UPDATE

Testing aside, passing in dependent objects also makes your code more explicit, flexible and extensible. To quote the blog post that I linked to:

When collaborator construction is mixed with initialization, it suggests that there is only one way to configure the class, which closes off reuse opportunities that might otherwise be available.

In your example, you can only create people that have "English" as a language. But what about when you are wanting to create someone who speaks "French". I can't define that.

As for creating the objects and passing them in, that is the whole purpose of the Factory pattern http://www.oodesign.com/factory-pattern.html. It would create the dependencies and inject them for you. So you would be able to ask it for the object that would be initialized in the manner that you want. The Person object should not have to decide what it needs to be.

Upvotes: 12

Touch
Touch

Reputation: 1491

Most of these answers seem reasonable. But I always do what you do to. If I am testing it, then I either have a setter or just set it to something (seeing that you "mother_language" is public).

But I personally think it depends on what you are doing. From the code that I am seeing posted by others, if your class instantiates objects, the constructor should always take some parameters.

What if I want to create an object that instantiates another object where that inner object instantiates another and so on? That seems like I will have a lot on my hands.

To me, that statement seems like he's saying, "don't use camelCase, use an under_score". I think you should do it whatever way works for you. After all, you do have your own way of testing, don't you?

Upvotes: 2

arraintxo
arraintxo

Reputation: 484

This is more a testability issue than a right/wrong thing, if you create an object on your constructor you will never be able to test your class independently.

If your class needs lots of injections on the constructor, you can allways use a factory method to inject those objects.

In this way, you will be free to mock any of those injected objects to really test your own class.

<?php

class Person {
  public $mother_language;

  // We ask for a known interface we don't mind the implementation
  function __construct(Language_Interface $mother_language) 
  {
    $this->mother_language = $mother_language
  }

  static function factory()
  {
    return new Person(new Language('English'));
  }
}

$person = Person::factory();
$person = new Person(new Language('Spanish'); // Here you are free to inject your mocked object implementing the Language_Interface

Upvotes: 4

Anyone
Anyone

Reputation: 2835

I think it depends on the situation. If you look at Doctrine 2 for example, it's recommended to put your values always to null and set your values in the constructor. This because Doctrine 2 skips instantiating the class when it's retrieved from your database.

It's perfectly acceptable to instantiate a new DateTime or ArrayCollection in there, you could even add default relational objects. I personally like Injecting your dependencies, keeps your code really easy to test and modify with little effort, but some things just make way more sense to instantiate in your constructor.

Upvotes: 3

Lightness Races in Orbit
Lightness Races in Orbit

Reputation: 385134

The original comment seems silly to me.

There's no reason to be afraid of classes that manage resources. Indeed, the encapsulation that classes provide is perfecty apt for this task.

If you restrict yourself to only ever pushing in resources from the outside, then what shall manage those resources? Spaghetti procedural code? Yikes!

Try to avoid believing everything you read on the internet.

Upvotes: 7

Related Questions