Are protected constructors considered good practice?

I'm writing some little helper classes to handle trees. Basically, I have a node and a special root node that represents the tree. I want to keep it generic and simple. This is part of the code:

<?php

class Tree extends TreeNode{
    public function addById($node_id, $parent_id, $generic_content){
        if( $parent = $this->findNodeById($parent_id) ){
            $parent->addChildById($node_id, $generic开发者_运维问答_content);
        }
    }
}

class TreeNode{
    public function __construct($node_id, $parent_id, $generic_content){
        // ...
    }

    protected function addChildById($node_id, $generic_content){
        $this->children[] = new TreeNode($this->node_id, $node_id, $generic_content);
    }
}

$Categories = new Tree;
$Categories->addById(1, NULL, $foo);
$Categories->addById(2, NULL, $bar);
$Categories->addById(3, 1, $gee);

?>

My questions:

• Is it sensible to force TreeNode instances to be created through TreeNode::addById()?
• If it's so, would it be good practise to declare TreeNode::__construct() as private/protected?

I think that in certain cases it does make sense to control the construction of objects and hide the public constructor.

This is true of your code: it's useful for the Tree class to control how it's child TreeNodes are created and initialized because it needs to control where nodes are added in the tree hierarchy.

Having this control over object construction is especially important if the relationship between the classes is such that the one has information about the other.

For example: if you changed your implementation slightly and allowed the Tree class to manage the node IDs for all nodes in the tree (you might store these in an array within the Tree class). In this case it would be very compelling to have Tree control how TreeNodes are created and initialized and doing this through a method on your Tree class makes perfect sense.

• Is it sensible to force TreeNode instances to be created through TreeNode::addById()?
• If it's so, would it be good practise to declare TreeNode::__construct() as private/protected?

If you want to force TreeNode to be created through TreeNode::addById(), the only sensible path is to make the TreeNode::__construct() private or protected (both would work in this case, but private would probably be better since it would force the subclasses to use ::addChildById).

As to whether it's sensible to force TreeNode instances to be created through TreeNode::addById(): it is, the alternative would be to transfer TreeNode::addById()'s logic to the constructor. While possible in this case, factory methods are generally more versatile.

Note, however, that as it is now, and since calling the parent constructors is not required in PHP, you can create (a subtype of) TreeNode objects by creating Tree objects. You should consider adding a private constructor to Tree to avoid instantiation.

Correction: although it's true that calling the parent constructors is not required in PHP, it's also true that there's an implicit call to the parent constructor if no constructor is specified in the subclass; so as it is right now, PHP would try to call TreeNode's parent constructor and fail when directly instantiation a Tree object.