Reputation: 4446
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:
ArrayAccess
interface) of objects), and this class would be used in another class, that has such a list of objects,DateTime
objects,include
(or autoload) dependant class, one should have no problem with errors,because dependant objects can be very large number, passing all of them to the class constructor can be very long and not clear, example
$color = new TColor('red'); // do we really need these lines?
$vin_number = new TVinNumber('xxx');
$production_date = new TDate(...);
...
$my_car = new TCar($color, $vin_number, $production_date, ...............);
as I was "born" in Pascal, then in Delphi, I have some habits from there. And in Delphi (and FreePascal as its competitor) this practice is very often. For example, there is a TStrings
class that handles array of strings, and to store them it does not use array
s but another class, TList
, that provides some useful methods, while TStrings
is only some kind of interface. The TList
object is private declared and has no access from outside but the getters and setters of the TStrings
.
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
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
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
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
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
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
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
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