Zend Framework and OOP Best practices

[aranworld]

I am by no means an expert on MVC or object oriented programming, but as I have been going through the Zend Framework tutorial, something strikes me as being odd and I was hoping one of the experts here could clear it up for me.

In the framework the following URLs would all open up the same controller:

http://www.google.com/Display/News/
http://www.google.com/Display/Groups/
http://www.google.com/Display/Images/

The controller would be structured thus:

class DisplayController
{
    function Groups()
    {
        // code required for displaying Groups
    }
    function News()
    {
        // code required for displaying News
    }
    function Images()
    {
        // code required for displaying images
    }
} 

But wouldn't a true OOP design instead be set up more like this:

abstract class DataIndex {
    function getDataRange()
    {
        // something to parse the url for what range of data to get
    }
    function getResultSet()
    {
        // query for obtaining result set
    }

    function displayResultSet()
    {
        // code to display a result set
    }
} 

class Groups extends DataIndex{
    // filled with any possible method overrides
    // specific to Groups
} 

class News extends DataIndex{
    // filled with News specific method overrides
    // if necessary
} 

[akrabat]

I would group differently:

http://www.example.com/news/list/
http://www.example.com/news/display/

http://www.example.com/groups/display

http://www.example.com/images/display
http://www.example.com/images/gallery

etc.

To me, your DataIndex class feels like the model in the MVC achictecture and so I would use it in the controller functions for providing the data that the view class needs for the display.

[Ezku]

Most likely you would want an URL scheme of the form google.com/news, but even going by the above scheme wouldn't mean you should code like you presented it. Actions related to each other are grouped in controllers thusly:

class NewsController extends PageController
{
   function index() { ... }
   function archive() { ... }
   function create() { ... }
   function delete() { ... }
   function update() { ... }
} 

I'm afraid you're quite lost indeed. Before you get any more confused, you should get acquainted with the kind of MVC and routing you're dealing with: Symfony has a nice writeup and I'm quite sure you'll be able to find something readworthy over at Rails, the inspiration to all of these implementations.

[aranworld]

I should have said that I am responding to the php architect tutorial and that is where I got the whole Display/Groups/, Display/Images/ idea from.

I agree with you entirely that ideally it should be:

Groups/Display/
News/Display/
Images/Display/

If that tutorial is correct, however, the Zend Framework wants it to be

Display/Groups/
Display/News/
Display/Images/

Which would all open an accompanying DisplayController class.

I guess what I am really wondering is if the tutorial I linked to above represents a good MVC system, or is is he not really using the framework correctly?

[aranworld]

Ezku, we are saying the same thing, and I appologize if my first post didn't make that clear. Based on your post, I would imagine that you would have the same issue with the php architect tutorial as I did?

[Ren]

/add/news
/add/comment

does seem the wrong way around in that article.
ZF deconstructs the path as /controller/method, and definately don't want an "add" c ontroller.

In you particular situation, I don't think you need the "Display" part either. The default action/method called on the News Controller should just display the news, imo

[33degrees]

I haven't looked into ZF too much, but I do know that in Rails and most of the rails inspired frameworks, it's trivial to change the URL mapping from controller/action/id to action/controller/id.

In any case, looking at the article in question, the author has definitely gone about his controllers and actions the wrong way around:

• It's counter to the usual OOP way of Objects being nouns and Methods being verbs.
• It's poor groundwork for future development, as adding functionality (such as a forum) would mean adding methods to the existing classes, instead of adding a new controller.
• He's had to resort to using __call and things like the redirect on the indexAction of the AddController, which is pretty inelegant IMO.

So a big thumbs down to this article from me. I know Chris Shifflet (the author of the article) frequents this forum, so maybe he'd like to explain why he chose this method instead of the alternative?

[shiflett]

So a big thumbs down to this article from me. I know Chris Shifflet (the author of the article) frequents this forum, so maybe he'd like to explain why he chose this method instead of the alternative?

Sorry to disappoint. :-)

To answer your question, it was a mistake, not a design decision. I'll correct it in the next update, and hopefully it doesn't ruin the tutorial for you...

[Ezku]

Ezku, we are saying the same thing, and I appologize if my first post didn't make that clear. Based on your post, I would imagine that you would have the same issue with the php architect tutorial as I did?

Oh, right. Seems like I have to apologize as well. :) Yes, I find the grouping extremely odd. Not the URLs, necessarily - oftentimes it makes a lot more sense to group them "backwards" - but the way how the controllers are organized irks me. There may be advantages to such a setup but the writer sure doesn't seem to elaborate on them. From a code organizing point of view, grouping by sections instead of functionality would make a lot more sense.

Looking at the credits, Chris Shiflett is a familiar character here at Sitepoint. And, looking at the previous post I missed for not reading to the end of the thread before posting, we seem to have his take on the issue already. Yay. I subscribed to your RSS feed, Chris. :)

[Overunner]

Regarding ZF, if you have say, a NewsController:

class NewsController extends PageController
{
   function index() { ... }
   function archive() { ... }
   function create() { ... }
   function delete() { ... }
   function update() { ... }
} 

Obviously, not everyone should be able to invoke the create, update, delete methods. So how is the best way to limit the access to these methods (so that only admins may invoke them for example)? Should I create my own dispatcher + router (+ plugins)...or will the upcoming release of ZF be able to handle this internally?

Or should I create an AdminController and have all the "secure" methods there, and do the access control when I create the AdminController?

[kyberfabrikken]

Oh ... while we're at the controllers in ZF, I have a (I hope) naive question. The module/command/arguments mapping is fine for simple table-oriented applications, but how would I go around adding a submodule ? Say I have a catalog of commodities, and each commodity has one or more images. The images are aggregates of the commodity - Eg. There's a foreignkey to the commodities pkey. So the most natural URL would be something like /commodities/160/images/ to list all images for commodity #160
This seems like a fairly common need - how is it this solved ? How is it solved in RoR, which seems to be the rolemodel ?

[Ren]

Oh ... while we're at the controllers in ZF, I have a (I hope) naive question. The module/command/arguments mapping is fine for simple table-oriented applications, but how would I go around adding a submodule ? Say I have a catalog of commodities, and each commodity has one or more images. The images are aggregates of the commodity - Eg. There's a foreignkey to the commodities pkey. So the most natural URL would be something like /commodities/160/images/ to list all images for commodity #160
This seems like a fairly common need - how is it this solved ? How is it solved in RoR, which seems to be the rolemodel ?

It currently isn't, the Router in ZF is hardcoded, its always

just
controller [use default action]
controller/action/(optional parameters) [use action]

There are comments in the ZF code that indicates a more flexible routing solution is in the pipeline.

If you want something more RoR like see my post
http://www.sitepoint.com/forums/show...0&postcount=27

Uses patterns (: for dynamics, * for wildcards) much like RoR, and with the requirements it compiles to a regexps form for matching with incoming requests.

PS. Ignore how it creates the controller, once its pulled apart a url, I wasn't to happy with the method used there, and currently gone with a DI style container for registering controllers with, and passing that to the Routes object.

[Ezku]

Say I have a catalog of commodities, and each commodity has one or more images. The images are aggregates of the commodity - Eg. There's a foreignkey to the commodities pkey. So the most natural URL would be something like /commodities/160/images/ to list all images for commodity #160
This seems like a fairly common need - how is it this solved ? How is it solved in RoR, which seems to be the rolemodel ?

I would imagine, since there is only a single level of pagecontrollers, that this would be dealt with by creating a route such as { '/commodities/:commodity/images/:action', :controller => 'images' } or however you saw fit.

PS. Ignore how it creates the controller, once its pulled apart a url, I wasn't to happy with the method used there, and currently gone with a DI style container for registering controllers with, and passing that to the Routes object.

Why does the Routes object have anything to with controllers? Is it like a combined route interpreter + dispatcher? Tell us more about your solution. :)

[Ren]

Why does the Routes object have anything to with controllers? Is it like a combined route interpreter + dispatcher? Tell us more about your solution.

Doesnt do the dispatching, just calls the gets an object from the container based on the key extracted from the url/route, and returns it. The FrontController dispatches.

Still not quite happy with it, as the container does some magickery.
The FC expects a object implementing interface { function execute($context); }

So to allow having

class NewsController
{
      function add($context) { ... }
      function delete($context) {... }
      function edit($context) { }
} 

It does some adapting of the controller, based on the action parameter extracted from the route.

class .. { 
function __construct($controller, $method) { $this->controller = $controller; $this->method = $method; }
function execute($context) { $this->controller->{$this->method}($context); }
} 

[33degrees]

Sorry to disappoint. :-)

To answer your question, it was a mistake, not a design decision. I'll correct it in the next update, and hopefully it doesn't ruin the tutorial for you...

I'm happy to hear that you're updating the article, then

[kyberfabrikken]

If you want something more RoR like see my post
http://www.sitepoint.com/forums/show...0&postcount=27

Uses patterns for dynamics, * for wildcards) much like RoR, and with the requirements it compiles to a regexps form for matching with incoming requests.

That's quite nice. I only skimmed the code, but I'll have a deeper look when I get some time off.
One thing bothers me with this patterns-based approach to routing. They are fairly static - they don't relate to the applications state. I recently tumbled with the same issue, and my solution was to skip the mapper, and instead let a set of nested controllers (intercepting filter if you want) take care of each part of the url as they see fit. Something like :

abstract class Context
{
    protected $env = Array();

    abstract function getEnv($property);
}

class HttpContext extends Context
{
    protected $response;

    function __construct() {
        $this->env =& $_SERVER;
        $_SERVER['RELATIVE_REQUEST_URI'] = ltrim(preg_replace("~^(".preg_quote(dirname($_SERVER['PHP_SELF']), "~").")~", "", $_SERVER['REQUEST_URI']), "/");
    }

    function getEnv($property) {
        return $this->env[$property];
    }

    function setResponse($response) {
        $this->response = $response;
    }

    function out() {
        echo $this->response;
    }
}

abstract class Controller extends Context
{
    protected $context;

    function __construct(Context $context) {
        $this->setContext($context);
    }

    function setContext(Context $context) {
        $this->context = $context;
    }

    function getEnv($property) {
        if (array_key_exists($property, $this->env)) {
            return $this->env[$property];
        }
        return $this->context->getEnv($property);
    }

    abstract function execute();

}

class RouteController extends Controller
{
    public $commands = Array(
        'index' => 'Action_Default_Index',
        'edit' => 'Action_Default_Edit',
        'create' => 'Action_Default_Create',
        'delete' => 'Action_Default_Delete',
    );
    public $default = "index";
    public $notFound = "Action_NotFound";

    function execute() {
        $uri = $this->context->getEnv('RELATIVE_REQUEST_URI');
        if (preg_match('~^([^/?#]+)(.*)$~', strtolower($uri), $matches)) {
            if (isset($this->commands[$matches[1]])) {
                $command = $this->commands[$matches[1]];
                $this->env['RELATIVE_REQUEST_URI'] = ltrim($matches[2], "/");
            } else {
                $command = $this->notFound;
            }
        } else {
            $command = $this->commands[$this->default];
        }
        $child = new $command($this);
        return $child->execute();
    }
}

class Dummy extends Controller
{
    function execute() {
        return get_class($this->context).":".get_class($this).":".$this->context->getEnv('RELATIVE_REQUEST_URI');
    }
}

class Action_Default_Index extends Dummy {}
class Action_Default_Create extends Dummy {}
class Action_Default_Edit extends Dummy {}
class Action_Default_Delete extends Dummy {}


class Commodities extends RouteController
{
    function __construct(Context $context) {
        parent::__construct($context);
        $this->commands['images'] = 'Images';
    }
}

class Images extends RouteController
{
}

class Application extends RouteController
{
    public $commands = Array(
        'commodities' => 'Commodities',
    );
    public $default = "commodities";
}

$context = new HttpContext();
$dispatcher = new Application($context);
$context->setResponse($dispatcher->execute());
$context->out(); 

Sorry no tests, this is just a stripped down version of a bigger codebase. Try the following paths to get an idea :

/
/commodities
/commodities/index
/commodities/create
/commodities/images
/commodities/images/create
/commodities/images/edit/160

(You'll obviously need a .htaccess to make it work)

[Ren]

That's quite nice. I only skimmed the code, but I'll have a deeper look when I get some time off.
One thing bothers me with this patterns-based approach to routing. They are fairly static - they don't relate to the applications state. I recently tumbled with the same issue, and my solution was to skip the mapper, and instead let a set of nested controllers (intercepting filter if you want) take care of each part of the url as they see fit.

How do you go about generating urls?
Eg. You want a url that'll get to the ImageControlller, with this image id.

[kyberfabrikken]

How do you go about generating urls?
Eg. You want a url that'll get to the ImageControlller, with this image id.

URL-routing and URL-building are obviously tightly integrated. I would build URL's in the same manner as I map the request - iteratively. Each context will have a method url($href) which would return a URL to a resource, relative to that context.
This is an expansion of the previous mock which shows how it works :

abstract class Context
{
    protected $env = Array();

    abstract function getEnv($property);

    abstract function url($href = "");
}

class HttpContext extends Context
{
    protected $response;

    function __construct() {
        $this->env =& $_SERVER;
        $_SERVER['RELATIVE_REQUEST_URI'] = ltrim(preg_replace("~^(".preg_quote(dirname($_SERVER['PHP_SELF']), "~").")~", "", $_SERVER['REQUEST_URI']), "/");
    }

    function getEnv($property) {
        return $this->env[$property];
    }

    function setResponse($response) {
        $this->response = $response;
    }

    function out() {
        echo $this->response;
    }

    function url($href = "") {
        return dirname($_SERVER['PHP_SELF'])."/".$href;
    }
}

abstract class Controller extends Context
{
    protected $context;

    function __construct(Context $context) {
        $this->setContext($context);
        $this->setEnv('URI_COMMAND', "");
    }

    function setContext(Context $context) {
        $this->context = $context;
    }

    function getEnv($property) {
        if (array_key_exists($property, $this->env)) {
            return $this->env[$property];
        }
        return $this->context->getEnv($property);
    }

    function setEnv($property, $value) {
        $this->env[$property] = $value;
    }

    abstract function execute();

    function url($href = "") {
        if ($this->getEnv('URI_COMMAND') == "") {
            return $this->context->url($href);
        } else {
            return $this->context->url($this->getEnv('URI_COMMAND')."/".$href);
        }
    }
}

class RouteController extends Controller
{
    public $commands = Array(
        'index' => 'Action_Default_Index',
        'edit' => 'Action_Default_Edit',
        'create' => 'Action_Default_Create',
        'delete' => 'Action_Default_Delete',
    );
    public $default = "index";
    public $notFound = "Action_NotFound";

    function execute() {
        $uri = $this->context->getEnv('RELATIVE_REQUEST_URI');
        $uri_command = "";
        if (preg_match('~^([^/?#]+)(.*)$~', strtolower($uri), $matches)) {
            if (isset($this->commands[$matches[1]])) {
                $command = $this->commands[$matches[1]];
                $this->env['RELATIVE_REQUEST_URI'] = ltrim($matches[2], "/");
                $uri_command = $matches[1];
            } else {
                $command = $this->notFound;
            }
        } else {
            $command = $this->commands[$this->default];
            $uri_command = $this->default;
        }
        $child = new $command($this);
        $child->setEnv('URI_COMMAND', $uri_command);
        return $child->execute();
    }
}

class Dummy extends Controller
{
    function execute() {
        return get_class($this->context).":".get_class($this).":".$this->context->getEnv('RELATIVE_REQUEST_URI');
    }
}

class Action_Default_Index extends Dummy {}
class Action_Default_Create extends Dummy {}
class Action_Default_Edit extends Dummy {}
class Action_Default_Delete extends Dummy {}

class MenuRouteController extends RouteController
{
    function execute() {
        $html = "<div style='border:1px solid black;padding:1em'>";
        $html .= "<ul style='margin-left:0'>";
        foreach (array_keys($this->commands) as $command) {
            $html .= "<li style='display:inline'><a href='".$this->url($command)."'>".$command."</a></li>\n";
        }
        $html .= "</ul>";
        $html .= parent::execute();
        $html .= "</div>";
        return $html;
    }
}

class Commodities extends MenuRouteController
{
    function __construct(Context $context) {
        parent::__construct($context);
        $this->commands['images'] = 'Images';
    }
}

class Images extends MenuRouteController
{
    function __construct(Context $context) {
        parent::__construct($context);
        $this->commands['index'] = 'Images_Index';
    }
}

class Images_Index extends Controller
{
    function execute() {
        return "<a href='".$this->context->url("edit/160")."'>edit image#160</a>";
    }
}

class Application extends MenuRouteController
{
    public $commands = Array(
        'commodities' => 'Commodities',
    );
    public $default = "commodities";
}

$context = new HttpContext();
$dispatcher = new Application($context);
$context->setResponse($dispatcher->execute());
$context->out(); 

The URL-method is quite primitive in this example. I have expanded it further to contain state too. For example a list-controller would have state for sort/order and possibly paging. Theese would be relevant to the controller and any child-controllers, but not for parents, so it could append the state to the url.

[ClickHeRe]

Regarding ZF, if you have say, a NewsController:

class NewsController extends PageController
{
   function index() { ... }
   function archive() { ... }
   function create() { ... }
   function delete() { ... }
   function update() { ... }
} 

Obviously, not everyone should be able to invoke the create, update, delete methods. So how is the best way to limit the access to these methods (so that only admins may invoke them for example)? Should I create my own dispatcher + router (+ plugins)...or will the upcoming release of ZF be able to handle this internally?

Or should I create an AdminController and have all the "secure" methods there, and do the access control when I create the AdminController?

In my personal framework I have "filters" that kick in in each stage.

One before the frontcontroller dispatches to the pagecontroller to check if the user has access to that page, else redirect to somewhere he does. Next part is just before th action is dispatched, another filter checks if the user has permission to that action. If he doesn't, he's redirected to an allowed action/page.

[Dr Livingston]

> In my personal framework I have "filters" that kick in in each stage.

Me too... Me too...

But on a Page Controller level, not a Front Controller level; I don't currently use a Front Controller you understand

[kyberfabrikken]

For this case, I wouldn't put the permissions on the controller-level, but push it to the model.
I have generic CRUD-controllers, which are allowed by anyone to execute, but inside the action, the model-component is queried for permissions. Eg. :

class Action_Default_Edit
{
    function execute() {
        ...
        $permission = $resource->getResourcePermission($identity);
        if (!$permission->queryPrivilege(NODE_PRIVILEGE_WRITE)) {
            throw new NotAllowedException();
        }
    }
} 

Most datatypes will have a per-class set of permissions, but this interface also allows me to use a finer granularity if needed.

[Dr Livingston]

Yes...

I had thought of that as well, but decided against it, and it's down to granuality as you say; For example the decoration of a given action would be solely to validate that the user in question can do something, such as edit a document;

Then within the action it's self, ie between

public function execute( Context $context, IDataspace $dataspace ) {
// ...
} 

I would have another filter or rule, to validate that the user can actually edit that given content it's self, as such that the user in question is either,

a) The original author, or
b) The user has higher or greater priveleges/permissions of the original author, to do the edit,

For example, the original author of the content could have the privelege level of Author, whereas the user requesting the edit has a higher privelege level, such as Publisher, shown below for example,

01 Administrator 01 00
02 Publisher     02 01
03 Author        03 02
...

So, if a users given Level (third parameter) is lower than the opposing users Level, they have overall control of the action requested - that way a Publisher can authorise if and when a document is ready for publication, since the author cannot automatically publish as such, for obvious reasons

[Dr Livingston]

Kyber,

> have generic CRUD-controllers, which are allowed by anyone to execute,

Such as for example, either

class EditNewsAction extends Action {}
class CreateNewsAction extends Action {}
class ModifyNewsAction extends Action {}
// or...

class EditNewsAction extends EditAction {}
class CreateNewsAction extends CreateAction {}
class ModifyNewsAction extends ModifyAction {} 

By generic, in reference to above examples, which route do you take? Would, or do you see any benifit of either sharing the responsibilities between a common abstraction (ie Action) or putting the responsibilities to separate abstractions (ie *Action)?

If this is going off topic, I'm sorry for doing so, but...

[kyberfabrikken]

The latter ... But per default I associate to the model through composition, not inheritance:

class DefaultEditAction extends Action {
    function __constructor($model) {
        $this->model = $model;
    }
} 

Should I need a bit more customization, I'd just extend it as in your example #2.