What defines a valid object state?

Question

I am reading an article about constructors doing too much work. One paragraph reads

In the object oriented style, where dependencies tend to be inverted, the constructor has a different and more Spartan role. Its only job is to make sure that object initializes into a state where it satisfies its basic invariants (in other words, it makes sure that the object instance starts off in a valid state and nothing more).

Here is a basic example of a class. On creation of the class I pass in the HTML which needs parsed to then set the class properties.

OrderHtmlParser
{
    protected $html;

    protected $orderNumber;

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

    public function parse()
    {
        $complexLogicResult = $this->doComplexLogic($this->html);

        $this->orderNumber = $complexLogicResult;
    }

    public function getOrderNumber()
    {
        return $this->orderNumber;
    }

    protected function doComplexLogic($html)
    {
        // ...

        return $complexLogicResult;
    }
}

I'm calling it using

$orderparser = new OrderHtmlParser($html);
$orderparser->parse()
$orderparser->getOrderNumber();

I use a parse function because I dont want the constructor to be doing any logic as both the article above and this article state this is terrible practice.

public function __construct($html)
{
    $this->html = $html;
    $this->parse(); // bad
}

However if I don't use the parse method, then all my properties (in this example just the one) would return null.

Is this known as an object in an 'invalid state'?

Also, it somewhat feels like my parse method is an initialise function in disguise, which is also deemed bad by the other article (although im not sure if that is only when a constructor calls that method, when it is manually called or both). Regardless, the initialise method is still doing some complex logic prior to setting a property - which needs to happen before the getters can be reliably called.

So either I am misunderstanding these articles or these articles are pushing me to think that maybe my overall implementation of this simple class is incorrect.

Answer 1

Generally, it's a code smell to perform work in a constructor, but the reason behind the practice has more to do with the programming language than an opinion about best practices. There are real edge cases that will introduce bugs.

In some languages derived classes have their constructors executed from the bottom up, and in other languages from the top down. In PHP they are called from top to bottom and you can even stop the chain by not calling parent::__construct().

This creates unknown state expectations in base classes, and to make matters worse PHP allows you to either call parent first or last in a constructor.

For Example;

class A extends B {
     public __construct() {
           $this->foo = "I am changing the state here";
           parent::__construct(); // call parent last
     }
}

class A extends B {
     public __construct() {
           parent::__construct(); // call parent first
           $this->foo = "I am changing the state here";
     }
}

In the above example class B has it's constructor called in different orders and if B was doing a lot of work in the constructor, then it might not be in the state the programmer was expecting.

So how do you solve your problem?

You need two classes here. One will contain the parser logic and the other the parser results.

class OrderHtmlResult {
      private $number;
      public __construct($number) {
            $this->number = $number;
      }
      public getOrderNumber() {
            return $this->number;
      }
}

class OrderHtmlParser {
      public parse($html) {
          $complexLogicResult = $this->doComplexLogic($this->html);
          return new OrderHtmlResult($complexLogicResult);
      }
}

$orderparser = new OrderHtmlParser($html);
$order = $orderparser->parse($html)
echo $order->getOrderNumber();

In the above example you could have the parse() method return null if it fails to extract the order number, or throw an example. But neither class ever enters into an invalid state.

There is a name for this pattern, where a method yields another object as the result in order to encapsulate state information, but I remember what it's called.