Say I have the following controller structure:
class Controller {
public function __construct(){
$this->accessControl();
}
}
class Account extends Controller {
public function __construct(User $user){
parent::__construct();
$this->user = $user;
}
}
How to require other devs to explicitly call parent::__construct()
in their child controllers? It contains critical stuff like access control etc
So far I decided to wrap all functions from parent constructor into init()
method which sets initialized
property to TRUE, then check this property in the router. If it's not TRUE - throw an exceptions.
public $initialized = false;
class Controller {
public function __construct(){
$this->init();
}
}
protected function init(){
$this->accessControl();
$this->initialized = true;
}
class Router {
public function process($path){
$controller = new User();
if(!$controller instanceof Controller || !$controller->initialized){
throw new Exception('Error');
}
}
}
Does it smell bad?
The same is needed in Symfony\Console Command, where constructor has to add command name and definition.
They handle it like this in single place where the command is added. So your approach to check controller in Router is the same. I'd got for it, rather then put the responsibility to abstract Controller or some reflection.
On the other hand, it looks accessControl()
should be checked on security layer level and not in router.
What framework do you use?
Could you decouple it from router by using EventSubscriber and framework event?
Yes, it smells bad.
How to require other devs to explicitly call parent::__construct() in their child controllers?
Well, firstly, you should trust your developers. The other question would be how can one be sure that all controllers have access control. To assure this you have to cover all your controllers with tests. So for each controller should be tested, that checks whether an unauthorized user can access it.
So far I decided to wrap all functions from parent constructor into
init()
method which setsinitialized
property to TRUE, then check this property in the router.
Actually, your init
function has no value, as you just moved code from __construct
(and at the end of the day now you have to ensure your developers call init
function -- see, only the name is changed;)). You can as well set initialized
in __construct
itself.
if(!$controller instanceof Controller || !$controller->initialized){ throw new Exception('Error'); }
So you will end up enforcing your developers to check whether or not the controller was initialized, in every place, it will be used in.
I guess you cannot make code foolproof. You have to trust developers to do the right thing and educate new team members. And of course, you should cover all your code with tests because in practice this is the only thing that ensures developers that they did the right thing.
There is a good podcast, that is tangible to your question: "Type Safety Roundtable with Ryan Tablada and Matt Machuga".