__construct vs init

[ahundiak]

Suppose you are designing a class which you know will be routinely extended. The __construct method accepts several arguments. You know the extended classes will usually want to do some sort of custom initialization.

Is it better to:
1. Have the extended class override the constructor keeping in mind that you need to keep the constructor signature intact.
or
2. Have the base constructor call an init() method once it is complete. The extended class will then override init().
or
3. A different approach

[TomB]

init() is generally considered bad practice.

What is "init" actually doing? Give the function a proper name and allow it to be called if required.

See http://misko.hevery.com/code-reviewe...oes-real-work/ for a detailed description.

[Michael Morris]

Suppose you are designing a class which you know will be routinely extended. The __construct method accepts several arguments. You know the extended classes will usually want to do some sort of custom initialization.

Is it better to:
1. Have the extended class override the constructor keeping in mind that you need to keep the constructor signature intact.
or
2. Have the base constructor call an init() method once it is complete. The extended class will then override init().
or
3. A different approach

3.

NO Function does more than one thing in polymorphic code. The construct function exists to set the start state of the object. This means attach the parameters to the members of the object. Nothing else.

The fact that you are considering having the object need an init strongly indicates an overworked construct function. Construct really doesn't do much. In PHP you can't do this

class MyClass {

    protected $foo = new Bar();

} 

The __construct function provides the workaround here.

class MyClass {

    protected $foo = null;

    public function __construct() {
        $this->foo = new Bar();
    }
} 

And honestly, that sort of thing is the only thing __construct functions should EVER do.

If you have tasks that you normally do as the object does then use a start function. Yes, this is clumsy

$myObject = new myClass();
$myObject->start(); 

But there are going to arise times sooner or later when you'll wish that you could do something between the instantiation of the object and the running of whatever those typical start up tasks are.

[TomB]

3.

The __construct function provides the workaround here.

class MyClass {

    protected $foo = null;

    public function __construct() {
        $this->foo = new Bar();
    }
} 

And honestly, that sort of thing is the only thing __construct functions should EVER do.

Giving a constructor the responsibility of object creation is bad practice. It makes for hidden dependencies and untestable code.

$bar = new Bar;
$foo = new Foo($bar); 

is far better.

[Michael Morris]

Giving a constructor the responsibility of object creation is bad practice. It makes for hidden dependencies and untestable code.

$bar = new Bar;
$foo = new Foo($bar); 

is far better.

Poorly followed through design patterns also lead to hidden dependencies and untestable code along with miles and miles of boilerplate code - so what was your point?

I have a table class that starts it's row and field objects at creation. Saves the writing of busybox boilerplate code over and over ad naseum. There are times that starting an object within another object is appropriate which is why the option exists. If you disagree write your own programming language which bars the practice.

[TomB]

So untestable code is a good thing then? That "busybox boilerplate code" is what makes it possible for you to take a piece of code and isolate it. Without it, you can't (easily) substitute the dependencies for mock objects during testing or replace them at runtime, completely forfeiting the benefits of polymorphism.

[Michael Morris]

So untestable code is a good thing then? That "busybox boilerplate code" is what makes it possible for you to take a piece of code and isolate it. Without it, you can't (easily) substitute the dependencies for mock objects during testing or replace them at runtime, completely forfeiting the benefits of polymorphism.

That boilerplate code is 99.99% of the time the code with the error in the first damn place in my experience. That's why I keep it to a minimum.

Further, if they are written correctly, you can test objects which have dependencies. One trick is to insure the dependency flows only one way. The Row class does not need the table class, the Field class needs neither.

[Czaries]

If you have tasks that you normally do as the object does then use a start function. Yes, this is clumsy

$myObject = new myClass();
$myObject->start(); 

But there are going to arise times sooner or later when you'll wish that you could do something between the instantiation of the object and the running of whatever those typical start up tasks are.

That's almost exactly option (2), only you have changed the function name from "init" to "start".

In my experience, it is often handy to have a separate "init" function that gets automatically run at the end of __construct in real-world code usage. This pattern - while certainly not ideal - does often allow you to extend and modify the class more safely. You don't have to worry about other stuff going on in the constructor (override only what you need to in "init") or dependencies changing (passed in constructor), and it helps discourage other developers from adding their own constructor parameters, which would break their code if you ever added another one later down the road. Most of the time, it's not necessarily about what's "right" in your code, but instead it's about what works best in the real world, even if that does ruffle the feathers of OO purists.

[Michael Morris]

In my experience, it is often handy to have a separate "init" function that gets automatically run at the end of __construct in real-world code usage. This pattern - while certainly not ideal - does often allow you to extend and modify the class more safely. You don't have to worry about other stuff going on in the constructor (override only what you need to in "init") or dependencies changing (passed in constructor), and it helps discourage other developers from adding their own constructor parameters, which would break their code if you ever added another one later down the road. Most of the time, it's not necessarily about what's "right" in your code, but instead it's about what works best in the real world, even if that does ruffle the feathers of OO purists.

The previous version of my framework has the controllers go through a list of tasks on startup. Near the end of last project with it I regretted doing that a lot - there where times I wanted to start controllers without doing their start tasks (often because another controller had already done those tasks and I was wanting to call controllers from other controllers).

I'm not wholly against the idea of taking actions at start but I ask myself these days - do I really need to do this every conceivable time this object starts? More often then not I can think of at least one case when I don't want to do that.

[TomB]

That boilerplate code is 99.99% of the time the code with the error in the first damn place in my experience. That's why I keep it to a minimum.

Further, if they are written correctly, you can test objects which have dependencies. One trick is to insure the dependency flows only one way. The Row class does not need the table class, the Field class needs neither.

If there's something odd going on in your table, you can't easily track down the source of the problem. It might be the field, it might be the row. There is no way to know. They are tightly coupled.

Even if you don't use TDD, surely the loss of polymorphism alone is worth abstracting the object creation code. What if you need a special type of Row in your table... you have to extend the whole table class to cater for it.

[Czaries]

So untestable code is a good thing then? That "busybox boilerplate code" is what makes it possible for you to take a piece of code and isolate it. Without it, you can't (easily) substitute the dependencies for mock objects during testing or replace them at runtime, completely forfeiting the benefits of polymorphism.

Fortunately, there is a happy medium. While it is better to use dependency injection, it can indeed get in the way sometimes, which is why things like factories, service locators, and dependency injection containers exist. Before you get there, however, you can take a middle road. You can go ahead and instantiate dependencies inside the object, and allow setters to override them. Zend Framework does this a lot. Something like:

class Table {
    protected $row;

    public function __construct() { }

    public function setRow(RowInterface $obj) {
        $this->row = $obj;
    }

    public function getRow() {
        if(null === $this->row) {
            $this->row = new Row();
        }
        return $this->row;
    }
} 

So the Row object is automatically supplied if not overridden, and can still be easily tested and mocked by supplying your own Row object that implements the common interface.

[Michael Morris]

Fortunately, there is a happy medium. While it is better to use dependency injection, it can indeed get in the way sometimes, which is why things like factories, service locators, and dependency injection containers exist. Before you get there, however, you can take a middle road. You can go ahead and instantiate dependencies inside the object, and allow setters to override them. Zend Framework does this a lot. Something like:

class Table {
    protected $row;

    public function __construct() { }

    public function setRow(RowInterface $obj) {
        $this->row = $obj;
    }

    public function getRow() {
        if(null === $this->row) {
            $this->row = new Row();
        }
        return $this->row;
    }
} 

So the Row object is automatically supplied if not overridden, and can still be easily tested and mocked by supplying your own Row object that implements the common interface.

Interestingly, that's exactly what I do. The Table class creates it's row interface by default and can be supplied one to use in preference of creating a new one similar to the above (the above is likely better than what I'm doing now to be honest).

[lastcraft]

Hi...

3.

class MyClass {

    protected $foo = null;

    public function __construct() {
        $this->foo = new Bar();
    }
} 

And honestly, that sort of thing is the only thing __construct functions should EVER do.

Forgive me, but can I throw a can of petrol on to the fire here?

First (as pointed out above) hard coding dependencies is going to turn out badly the first time you want to change that dependency.

The first use case is usually testing. If you test a class and it's dependences at the same time, the number of lines of code under test grows an order of magnitude. This slows your productivity down by roughly half the same factor (assuming you spend roughly half your time writing tests and the other half getting them to pass).

Except for small apps, this is just the kind of thing you don't want to do in the constructor.

You do want stuff to happen in the constructor besides this. Between the __construct() call and the init() call your object is in an indeterminate state. Any call on it will generate a bug. You want a bug factory?

The C++ crowd (who have a far more demanding job having to manage memory as well) have a phrase "construction is initialisation". In other words, if you have any set-up to do, do it in the constructor.

A useful point here is that if you go...

$working_at_last = new GoodLuckGettingThisUpAndRunning(); 

...and the constructor throws an exception, you never get an object in the variable $working_at_last. This is handy if the cause of the exception would leave the object useless and unrecoverable. If you your init() code contains such a possibility, move it to the constructor. Then your program is guaranteed never to see an object in that state. Stronger than strong typing - how cool is that?

The exception to all of this is lazy construction. If you are doing this I won't disturb you further. You need all the concentration you can get.

yours, Marcus

[Michael Morris]

Your condescension is overwhelming lastcraft, mind turning it down a notch or three? I wasn't even thinking about dependencies when I wrote that example.

[lastcraft]

Hi...

Oh, sorry. I didn't think I was being that condescending. Apologies. I was picking apart that lone statement and certainly not you.

You must admit that "And honestly, that sort of thing is the only thing __construct functions should EVER do." is a pretty strong statement placed right next to a hard coded dependency. What did you have in mind?

yours, Marcus

[Michael Morris]

Hi...

Oh, sorry. I didn't think I was being that condescending. Apologies. I was picking apart that lone statement and certainly not you.

You must admit that "And honestly, that sort of thing is the only thing __construct functions should EVER do." is a pretty strong statement placed right next to a hard coded dependency. What did you have in mind?

yours, Marcus

__construct is for getting the object to it's default state as you said. Whether having new Bar(); in the constructor is appropriate has as much to do with the nature of Bar as dependency concerns.

If Bar is little more than an stdClass with a few methods and no constructor of its own then the consequences of instantiating it from __construct will be minimal. The rule about constructors doing nothing except get set up runs through to all the children. But if Bar has to anything that could fail, particularly running a query or validate an input, it shouldn't be instantiated during construct, but it still may need to be taken in as an argument.

Arguments to a class make me worry because they mean binding code. Binding code means repetition, which breaks DRY, and provides a spot for errors to occur. Factory patterns can alleviate this but at the cost of overhead and complexity.

I also am a firm believer of code that dies often and hard in development. I develop with E_ALL + E_STRICT for that reason. If an error state cannot be naturally reached assert is used to test things. I use assert a LOT - I've found that one well placed assertion can stop hours or even days of agony.

I'm a fan of soft dependencies - by that I mean dependencies that can be changed or even suppressed if needed (as in a test environment).

There comes a point though where you have to trust your code. Trust is earned through testing of course but also through use - after all even the best tests can let odd corner cases get through. So if Bar is an object I've used for months without issue, I might consider putting it in a __construct. If it's a week or two old though the answer is naturally no - and hell no at that.

[lastcraft]

Hi...

Arguments to a class make me worry because they mean binding code. Binding code means repetition, which breaks DRY, and provides a spot for errors to occur. Factory patterns can alleviate this but at the cost of overhead and complexity.

Actually DI can eliminate a lot of the repetition straight away. Factories are often added as necessary anyway, so you won't add many. When it comes to sorting out older code, tangled dependencies cause far more harm than passing a few extra parameters around.

You will probably say I'm being condescending, but in my experience this advice is wrong. Passing dependencies is way simpler.

yours, Marcus

[Michael Morris]

Hi...



Actually DI can eliminate a lot of the repetition straight away. Factories are often added as necessary anyway, so you won't add many. When it comes to sorting out older code, tangled dependencies cause far more harm than passing a few extra parameters around.

You will probably say I'm being condescending, but in my experience this advice is wrong. Passing dependencies is way simpler.

yours, Marcus

You'll not convince me of that - not when I'm working with an old code base that required me to add two arguments to 9 files to add 1 stinking field to a bloody report. And that code has hidden dependencies to go along with the now 15 arguments per file.

God I hate this code. I'm risking my job by developing its replacement in secret because the boss, who isn't a coder at all, doesn't understand the whole mess can be replaced much cheaper than it could ever be maintained. Then again, I'll lose my job anyway if I can't keep up with his unrealistic expectations so it's triage. At least I still have two jobs in case one croaks, but the stress is getting to me.

[webaddictz]

You'll not convince me of that - not when I'm working with an old code base that required me to add two arguments to 9 files to add 1 stinking field to a bloody report.

How is that related to the problem being discussed, if I may be so bold? The fact that the software you're maintaining is written differently than what I would write, doesn't mean parameters are a bad sign, just that there are too many layers of indirection, and that the application is (probably) overcomplicated. Hiding the dependencies themselves isn't going to help you, in fact, quite the opposite, or at least - that's how I see it.