4

Recently, I saw a colleague of mine instantiate his classes in a constructor, so I started doing the same, like this:

class FooBar{
private $model1;
private $model2;
    public function __construct() {
        $this->model1=new Model1();
            $this->model2=new Model2();
    }
}

And now I'm starting to wonder, if maybe instantiating the models everywhere where they are needed may be better?

E.g., function foo() needs model1 and function bar() needs model2, but now both models are loaded.

So, the question: Is this the right way to instantiate other classes? Or should I just instantiate them when I need them in a function?

Janis Peisenieks
  • 4,938
  • 10
  • 55
  • 85
  • That depends on the context. If you are calling the methods more than one time, do you want to instantiate a new model every time or not? If you want to, is it really necessary? – Felix Kling Mar 06 '11 at 13:42

4 Answers4

3

Well, as always there is no one size fits all answer.

Most of the time, class FooBar aggregates $model1 and $model2 because it needs them to fulfill its function. In this scenario there's not much that FooBar can do unless it has objects in these variables, so it's the right thing to do to create them in the constructor.

Sometimes an aggregate object is not needed to perform a large part of class FooBar's function, and the construction of that object is an expensive operation. In this case, it makes sense to only construct it on demand with code like the following:

class FooBar {
    private $model1;
    private $model2;

    public function Frob() {
        $model = $this->getModel1();
        $model->frob();
    }

    private function getModel1() {
        if ($this->model1 === null) {
            $this->model1 = new Model1;
        }

        return $this->model1;
    }
}

However, that's only sometimes. If class FooBar needs $model1 for half of its operations and $model2 for the other half, this may indicate that FooBar is suffering from a case of "let's throw everything inside one class" and should be split into two classes instead.

Jon
  • 428,835
  • 81
  • 738
  • 806
  • Could all of this instantiating and testing, if a class has been instantiated be done in a seperate class? Like a global class instantiating class? Sounds a bit perverted, but it would make the instantiating more reusable. – Janis Peisenieks Mar 06 '11 at 13:55
  • @Janis: the testing, no. The instantiating, yes. You can either provide callbacks to the `FooBar` constructor that will return an appropriate model when called, or go all the way and use a Dependency Injection container (http://www.google.com/search?q=+php+dependency+injection+container). – Jon Mar 06 '11 at 13:58
  • Since Your answer contained more information that was valuable for me, I chose this as the accepted answer. – Janis Peisenieks Mar 07 '11 at 07:16
2

I would like to see these dependencies injected into the constructor as parameters.

rcravens
  • 8,320
  • 2
  • 33
  • 26
  • Yes, and if you need to create instances on the fly of other classes, then inject a factory that can create them. Dependencies should be supplied, not instantiated, so that you can replace them with mock instances for testing. – tvanfosson Mar 06 '11 at 13:46
1

You should actually be loading them when you need them otherwise a whole bunch of models that are not required (which may have their own constructors with more models loading!) will pop into memory every time you need a trivial operation done.

Don't create a new model unless you're sure you will be using them (e.g. models needed to localize and such)

JohnP
  • 49,507
  • 13
  • 108
  • 140
1

It is not exact science, and you should follow your instincts in how to organize the code.

If this approach gets unmaintainable, or you want to unit test it, dependency injection might come to the rescue.

But if you're doing simple scripts and development time is an important factor, the way you're doing it now is sufficient.

pestaa
  • 4,749
  • 2
  • 23
  • 32