I’m using a DI container to assemble my application, however one area I’m a little stuck on is action controllers.
Until run time it is impossible to know what dependencies a controller has because each action can have different requirements. The logical solution seems to be treating the DI container a bit like a service locator and just injecting it into the controller. It doesn’t really feel right to do this though.
I’d be interested to know how others have handled this and of there’s a general consensus on how this should be dealt with.
Now $page is a page controller. Not sure how this equates to Action.
Right now the Router pulls in the Request object, the necessary includes are handled in getControllerName() and $page is a functor (it handles the page request in construction and uses the view to paint it all in the constructor!). That’s possibly a little too much magic for public consumption :).
The Views class is just a factory/repository and uses the request too. I sometimes find myself using DI for dependencies, but letting a factory do the final instantiation.
I haven’t tried this second code block, but looking at it now I prefer it to the snippet above that I use in my pet projects. Hm…I might still hide Request.
What I like about this setup is that I can add models to the controller or the view constructors and DI just takes care of it. The MVC flavour is hidden (e.g. PassiveTemplate or not). The injector never escapes from index.php, but I guess this is essentially the same as Kyber’s.
>The Views class is just a factory/repository and uses the request too. I sometimes find myself using DI for dependencies, but letting a factory do the final instantiation.
Find myself in the same behavior. But probably with another different slant. I configure injector (all the magic you hide in wiring file) in the factory and than return the result object, like:
Agreed. Cohesion is still the best guide to constructing functions; it should do one and only one thing.
I’m not good with MVC, but I like factories simply for their ability to encapsulate the model in this case and delay binding time until the last possible moment, two best practices in one. If the situation really is as complicated as you mentioned elsewhere it is also possible to build a factory for factories. You could also use an abstract factory pattern. You declare an interface for the factory, and then an interface for each object that each factory creates. This may be an abstract class if you have implementation to automate across your factories. Then implement your factories and the objects they build according to the interfaces.
Not really, to switch the view you’d switch the controller. How can a view work without its controller?
Otherwise, each controller action needs to specify which view it’s going to use. By making the relationship 1:1 this only needs to be defined once.
Now, I have no idea how your own system works but it’s common in some “MVC” frameworks to do things like this:
(for ease I’m assuming a 1:1:1 relationship for these examples )
class UserController extends Controller {
public function showList() {
$users = $this->model->findAll();
$this->set('users', $users);
$this->render('list.tpl');
}
public function sortList($order) {
$users = $this->model->setSort($order);
$this->set('users', $users);
$this->render('list.tpl');
}
public function edit($id) {
$user = $this->model->findById($id);
$this->set('user', $user);
$this->render('form.tpl');
}
public function save() {
if ($this->model->save($_POST)) {
$this->render('success.tpl');
}
else {
$user = $this->model->findById($_POST['id']);
$this->set('user', $_POST);
$this->render('form.tpl');
}
}
}
Which is backwards. This controller has two responsibilities: editing and listing users, causing unecessary repeated code.
Isn’t this better:
class UserListView extends View {
public $template = 'list.tpl';
public function render() {
$this->set('users', $this->model->findAll());
}
}
class UserListController extends Controller {
public $view = 'UserList';
public function sort($order){
$this->model->setSort($order);
}
}
class UserEditView extends View {
public $template = 'form.tpl';
public function render() {
if ($this->model->isSaved()) {
$user = $this->model->getUser();
$this->set('user', $user);
}
else $this->template = 'success.tpl';
}
}
class UserEditController extends Controller {
public $view = 'UserEdit';
public function main($id) {
$this->model->id = $id;
}
public function save() {
$this->model->save($_POST);
}
}
Now, any controller actions which are added relate to a single view meaning they don’t have to specify which view they’re going to use and reduces the need for ‘default’ controller actions which take no parameters.
Assuming you have a front controller, you could let the front controller be aware of the container and use it to instantiate the handler. The handler, however, is purely DI style. So:
Often when doing simple lists I don’t actually need anything in the controller, so can be quite lazy and combine listing and editing. But I agree It’s better to have an empty controller than to mix those responsibilities.
Also I’m swinging toward the factory idea for models now. In most cases a controller will require a certain model under all circumstances, so it can just be injected in. If I can’t tell which model I need till runtime, a factory for the possibilities should do the trick.
I think I just need to start focusing more on convention rather than configuration.
DI should sort out the problem of requiring different factories. You should need at most 1 for models and one for views, DI should sort out the dependencies of those as they’re created.
One approach is to create the view when you create the controller.
interface controller {
public function getViewName();
public function setView(View $view);
}
$controller = $dic->create('MyController');
$view = $dic->create($controller->getViewName);
$controller->setView($view);
You could do the same with models but it’s useful for the controller to access multiple models*
*In MVC proper, each MVC triad is a 1:1:1 relationship, models can access other models. This can create a lot of pointless wrapping functions in models so I think it’s better to skip the middle man and allow the controller access to any model, of course this does limit portability of the triad. The controller-view relationship should remain 1:1, however.
This is what I do as well. I use a Kernel object (AppKernel, which is essentially a service locator with some additions) that is injected into the constructor of the controller, and a request object that is passed into the action that is called as the first parameter. The controller action returns a response to be displayed, which can be either be nothing (no content), a string (JSON, etc), a view object (HTML or XML template, etc.), or false (to trigger a 404). It’s a little different setup than most others I have seen (including those posted here) - because it requires an explicit return instead of implicitly pre-loading a view - but I like it better specifically because it doesn’t load anything automatically, and is easier for newbies to follow since it’s more clear what is going on.
The controller loads any model objects, mappers/entities, or other objects it needs directly from the kernel. I don’t like proxying this through an abstract or front controller (as kyberfabrikken suggested), because it’s not the job of a controller to know how to load or assemble objects, and it just results in more function calls and misdirection. That’s what your service locator is there for, so don’t be afraid of using it directly.
My approach looks something like this (Vehicle detail from AutoRidge):
<?php
class Module_Vehicle_Controller extends Alloy_Module_Controller_Abstract
{
/**
* View single vehicle record
* @method GET
*/
public function viewAction(Alloy_Request $request)
{
// Get vehicle record
$mapper = $this->kernel->mapper('Module_Vehicle_Model');
$vehicle = $mapper->first(array('year' => $request->year, 'make' => $request->make, 'model' => $request->model));
// Ensure vehicle exists
if(!$vehicle) {
return false; // Triggers 404 File Not Found
}
// View template - uses __toString to render content when response is sent
return $this->view(__FUNCTION__)
->set(array(
'title' => $request->year . " " . $vehicle->make . " " . $vehicle->model,
'vehicle' => $vehicle
));
}
}
Both model and view objects could be passed to the controller. I had considered a factory but I decided against it as essentially the DI container is a factory and each action could have different requirements and therefore use a different factory, which just seems very messy (unless I’m looking at it all wrong). The other thing of-course is by allowing the factory access to the container, the controller’s only dependency is a factory that with no clear dependencies, so we’re just moving the problem.
Is there any other ideas? I’m sure others have encountered this problem.
That’s what I do. You can 1) Hide this in the framework layer, so that the concrete controllers don’t normally interact with the container directly (If you use a front controller pattern, you can stick it there), 2) wrap the container in a factory, that only allows you to create controller instances. This is to prevent application level code from using the container as a registry.
What sort of things does your application controller need? Are they related to each other? e.g. always models? If that’s the case (and i can’t really imagine anything else you’d realistically want to pass to an application controller) then pass a factory for creating them to the controller using DI and let the factory handle the creation of the objects, and although it may (?) be a slight misuse of the pattern I see no real harm in letting the factory access the container to create the objects.
You can 1) Hide this in the framework layer, so that the concrete controllers don’t normally interact with the container directly
Do you mean setting the container to the controller and accessing it through get methods from an action e.g.
class ExampleController
{
public function IndexAction()
{
$a_model = $this->getModel('AModel');
$a_model->doSomething();
$this->view = $this->getView('Example');
$this->view->setAModel($a_model);
}
}
So this would essentially be using the container as a registry?
wrap the container in a factory, that only allows you to create controller instances.
I’m not really sure what you mean , how would this prevent the container being used like a registry?
The controller-view relationship should remain 1:1
TomB, why is this? Surely you’d what to allow view switching at runtime?
and reduces the need for ‘default’ controller actions which take no parameters.