Tell don't ask vs constructor doing work

Question

When you browse for the phrase "constructors must not do work", then in various blog posts you will find the advice to not let the constructor do work. Despite this, I am having some trouble understanding why this is the case. Additionally, this popular post suggests to take such advice with a grain of salt.

I have an example of two implementations of the same situation. In the situation, a AFactory has a method createA, using a B. A needs a query result, that B produces. There are two ways to implement this:

Example 1:

class AFactory {
    public function createA(B $b): A {
        return new A($b->getQueryResult());
    }
}

class A {
    private $query_result;

    public function __construct(array $query_result) {
        $this->query_result = $query_result;
    }

    public function doFooWithQueryResult() {
        // Do something with query result
    }

    public function doBarWithQueryResult() {
        // Do something with query result
    }
}

In the first example, the factory fetches the query result and passes it to A's constructor. A then merely assigns the query result to the corresponding class property. However, there is one problem here: A does not verify if the query result is a valid data structure, i.e. an actual query result suited for A. It does not know where it came from. The responsibility for this validation has now leaked to the AFactory, and A has become very tightly coupled to AFactory. The other implementation resolves this issue, but then the constructor performs work. And apparently that is bad.

Example 2:

class AFactory {
    public function createA(B $b): A {
        return new A($b);
    }
}

class A {
    private $query_result;

    public function __construct(B $b) {
        $this->query_result = $b->getQueryResult();
    }

    public function doFooWithQueryResult() {
        // Do something with query result
    }

    public function doBarWithQueryResult() {
        // Do something with query result
    }
}

Answer 1

You wrote

A does not verify if the query result is a valid data structure, i.e. an actual query result suited for A.

directly followed by

It does not know where it came from

But these are two different things! A does not not need to "know where the data is coming from", but it can of course validate its input:

class A {
    private $query_result;

    public function __construct(array $query_result) {
        // makes tests, throws an exception if $query_result is not valid
        validateInput($query_result);  

        $this->query_result = $query_result;
    }
    // ...
}

IMHO input validation in the constructor is not counting as "actual work" which should be done somewhere else. Nevertheless the constructor does not need to call getQueryResult. That will keep A decoupled from B, which makes testing and reuse much easier.

Answer 2

What is A? This is the fundamental question in designing a class in OOP.

Most problems arise because of not asking this question at the outset and instead focusing on implementation details of the class. Your example gives me this impression.

Back to the question:

• Ask the question before doing any details. In other words you need to define what the class A is. Don't move to implementation before finalizing this question. Typically, one is tempted to think that once one starts coding, the class will come together somehow. Avoid this temptation.
• When asking this question, try to define a single, yes single responsibility for the class, not many. And this responsibility has to be clear not vague.
• Once you have made the definition, then it becomes much easier to see what properties this class has, because properties stem from its definition. For example, if you decide that your class is a Vehicle, then it must have attributes such as body, engine, tires, steerings etc and actions such as move, stop, turn, speed up, slow down, etc.
• It is important to do these steps before diving into coding, no matter how tempting that could be.

Now, to your question regarding constructors should not do the work:

My understanding for this rule (though not absolute as you have mentioned) is this: In your application, by the time you reach the point to construct an object of a class, the data for that construction should be already available and passed to constructor. For example, say in your web application, you gather user-entered data through a form to construct an object.

// Gather user information
email = email from form data
...

// Do data validation if necessary
if (name is valid)
if (email is valid) 
if (credit card is valid)
...

// Now ready to construct object
User user = new User(name, email, creditCard, ...)
...

Same thing goes if you need to get data from a database to be used for construction of an object, in you example A.

The idea is, in the context where you create an object, pre-construction work needs to be done there. For example, the above example could a Controller in a web application.

There is one thing that should be better done in the constructor, and that is the kind of work that logically is part of the definition of a class (Earlier explanation about the defining a class). For instance, in the web user example, if it is part of the business logic that each user be assigned a rank based on their credit limit, this kind of work is not suited in the controller, not perhaps in a service class, but in the business object itself. So, something like this could be done in constructor.

Answer 3

Class A shouldn't know what B does. Class A shouldn't even know what a query result looks like.

An improvement would be to have explicit named parameters - you know exactly what data you need to construct A - whether that comes from a query result or not.

I'd think about what could actually be wrong with the query result - it seems that if the query is wrong, this is more of an exceptional circumstance that should be treated as such - leading to "A" instances never being created.

More generally speaking as Doc Brown suggests, it's about reducing "coupling" - or how dependent a piece of code is on a different piece.

See here for a quick guide to some different ways code may be coupled. You'll see it's impossible to avoid in some way. You want your code to be as loosely coupled as possible so you can test and modify it independently.

If your constructor just takes named parameters - you don't need to do complicated work in it, except maybe assigning simple variables. The complicated work you do to you arrive at those parameters' values can then be ignored by A, or any dependency on A, making A really easy to unit test. That complicated work is also then a candidate for hiding behind an abstraction and unit tested too.