Could it then be named Phails?Quote:
Originally Posted by arborint
Printable View
Could it then be named Phails?Quote:
Originally Posted by arborint
I think that's a good idea, once we get things settled.Quote:
Originally Posted by arborint
I agree that real-world projects may create such needs - that's my experience aswell. Rather than solving it within the Mapper, I prefer to have multiple mappers, and then stick these on a stack/chain, so to keep responsibilities seperated.Quote:
Originally Posted by arborint
Yes, we probably should make the FrontController be an InterceptingFilter, so the user can stick other filters to it. Since I have the feeling that we have more or less come to a common ground on the basic FrontController, it may be the direction to move into now. Would you have the first swing at it ?Quote:
Originally Posted by arborint
Yes, the ViewHelper can be used to put the seperation back in.Quote:
Originally Posted by overunner
One thing to be aware of is that ViewHelper is a pretty vaguely defined term/pattern. It can mean 1) a Gateway to the Model, but it could also simply mean 2) a sub-view (~Widget). Or it could even be used as synonymous with PresentationModel. So if you try to read up on the pattern, have your guards up.
I have a personal preference for DispatcherView, over TemplateView, since it calls for fewer lines of code to begin with. The weakness of DispatcherView shows up when your view becomes increasingly complex - you will find that Controller-layer code begins to show up in your View, which of course is bad. The solution is to put this code in seperate ViewHelpers (The 1. meaning of the word). In the simplest form, the ViewHelper could thus simply be a function, which you call from within the DispatcherView.
Quite amazing to reach common ground in this forum. :) I really appreciate you keeping me honest and the code quality up.Quote:
Since I have the feeling that we have more or less come to a common ground on the basic FrontController...
A question I would ask everyone else reading this thread is if the "common ground" we claim to have reached is one that you see as useful? I guess I would ask programmers who currently use an OOP Front Controller if this code comes close to meeting their needs? And I would ask programmers interested in adding a Front Controller or MVC if they would use this code? Is his code a good example of a PHP Front Controller for the cadmiumgreens out there?I would be glad to, but I have a few questions and comments:Quote:
Yes, we probably should make the FrontController be an InterceptingFilter, so the user can stick other filters to it. ... Would you have the first swing at it ?
- Do we want Pre and Post filter chains? The Pre filter chain passing the Request along. The Post filter chain passing the Response along.
- Or just a single chain that you add objects in the order you want them executed and they all get both the Request and Response object passed to them?
- Several of the objects may want to know what the "action" is. You should be able to set the action parameter to anything you want (it defaults to 'action'). How is this configurable setting passed to the filters?
Just don't call it Phalus, though Phialis is pretty funnny -- PHP on Phialis ;)Quote:
Originally Posted by sweatje
I think its a really good start and comes close to meeting my needs but I wouldnt use the code yet and I dont think its a good example yet.Quote:
Originally Posted by arborint
I dont want to come off as too down about the code but rather offer some constructive critisism. Write tests for the code, it's in need of refactoring. If one of the goals is to be used as an example of a front controller the code should be very tight and its not there just yet.
Personally I would drop the prefix / suffix stuff.
Obvious refactorings after skimming the code is in the ServerPageMapper and ActionMapper classes. The untaint method is a copy/paste. The mapRequest methods are also very similar and could be merged I bet.
Id also not pass $map as an array to ActionLookUpMapper. Intead of :
Go with something like:PHP Code:$map = array(
'home' => array(
'file' => 'home.php',
'class' => 'home',
),
'foo' => array(
'file' => 'bar.php',
'class' => 'bar',
),
'bar' => array(
'file' => 'foo.php',
'class' => 'foo',
),
);
$controller =& new FrontController(new ActionLookupMapper($map, 'action/', 'home', 'action'));
$controller->execute(new Request());
Moving from an array of arrays to an array of objects will allow you to add logic to the actions rather than holding logic in the *Mapper classes I bet. Might be able to get down to just 1 mapper class.PHP Code:$mapper = new ActionLookupMapper('action/', 'home', 'action');
$mapper->addAction(new Action('home', 'home.php', 'home'));
$mapper->addAction(new Action('foo', 'bar.php', 'bar'));
$mapper->addAction(new Action('bar', 'foo.php', 'foo'));
$controller =& new FrontController($mapper);
$controller->execute(new Request());
Anyways.. just some suggestions.
It is encouraging that it "comes close to meeting my needs" and I agree with your high level assessment.Quote:
Originally Posted by Brenden Vickery
I believe kyberfabrikken and I (who have been doing the coding) have been pretty receptive to critisisms, so don't hold back.Quote:
Originally Posted by Brenden Vickery
I would agree. I also would welcome you to refactor any part of the code you think needs it and post it. I have also been hoping that someone would contribute some tests but so far no.Quote:
Originally Posted by Brenden Vickery
I added that because of the wide variablity in how people arrange directories and name files and classes. I believe strongly that all of these things should be configurable and that there should be as few constants as possible. By default it includes files in the current directory and directly uses the action value as the file and class name. I don't think it makes sense to force people into a file structure like a framework does.Quote:
Originally Posted by Brenden Vickery
Yes I have thought of creating a BaseMapper class as well. I made these minimal but they have already been refactored once in the last go round. Maybe you could post how you would refactor the mapper classes.Quote:
Originally Posted by Brenden Vickery
Maybe I should clarify how I intend the map to be used. This array maps action params onto file/class names. It could be loaded from an XML or INI file, or straight from a PHP file. It would contain many entries (one for each potential action param value) so I don't think I'd want the overhead of loading/parsing a map and then doing a hundred "$mapper->addAction(new Action($param, $file, $class));" calls. I used an array because I assumed that it would be easy to create that from any type of configuration file. I added the data field so a framework could add meta data (like access control) to each action.Quote:
Originally Posted by Brenden Vickery
If you wanted to do it your way on a page by page basis the following class would do it:And use it like this:PHP Code:class ActionMap {
var $map = array();
function addAction($param, $file, $class, $data=null) {
$this->map[$param]['file'] = $file;
$this->map[$param]['class'] = $class;
$this->map[$param]['data'] = $data;
}
function getMap() {
return $this->map;
}
}
Does this make sense to you and do you think it would be a worthwhile addition? We could put it in the file with the ActionLookupMapper.PHP Code:$map = new ActionMap();
$map->addAction('home', 'home.php', 'home');
$map->addAction('foo', 'bar.php', 'bar');
$map->addAction('bar', 'foo.php', 'foo');
$mapper = new ActionLookupMapper($map->getMap(), 'action/', 'home', 'action');
$controller =& new FrontController($mapper);
$controller->execute(new Request());
Thanks for you comments Brendan.
I look around at a number of examples (mostly Java of course). I picked a Filter Chain that passes itself to the filters so they can add to the chain if necessary.Quote:
Originally Posted by kyberfabrikken
I am assuming that the PHP script itself is the FilterManager adding filters, so all I did was a FilterChain class. FilterChain is a bad name because it is generic, so I called it FrontController and renamed FrontController to ActionDispatcher? Rename if you think it is not clear.Here is the class formerly know as FrontController:PHP Code:class FrontController
{
var $filters = array();
function FrontController() {
}
function addFilter(&$filter) {
$this->filters[] =& $filter;
}
function process() {
$request =& new Request();
$response =& new Response();
$i = 0;
while ($i < count($this->filters)) {
$this->filters[$i]->execute($request, $response, $this);
++$i;
}
$response->execute();
}
} // end class FrontController
And here is how it is called:PHP Code:class ActionDispatcher
{
var $mapper;
function ActionDispatcher(&$mapper) {
$this->mapper =& $mapper;
}
function execute(&$request, &$response) {
$dispatcher =& $this->mapper->mapRequest($request);
if (method_exists($dispatcher, 'execute')) {
$dispatcher->execute($request, $response);
}
}
} // end class ActionDispatcher
PHP Code:$controller =& new FrontController();
$controller->addFilter(new SomeFilter());
$dispatcher =& new ActionDispatcher(new ActionMapper('action/', 'home', 'action'));
$controller->addFilter($dispatcher);
$controller->process();
Some of the latest code examples I have seen come pretty close. However, I believe that some parts of it don't follow well-established OOP methods, like encapsulating algorithms in classes and template methods.Quote:
Originally Posted by arborint
It is completely unnecessary to distinguish between pre and post filters. When you pass a request and a response to a filter it can carry out any pre-filtering, then pass the request and response to a second filter and then carry out any post-filtering. Filters can even to decide not to pass the request on to the next filter, but to take some alternative action. For example, a filter checking for authentication credentials can pass the request on to the next filter when valid credentials are present, but redirect to some login page when they are not. I'd be happy to show some real world implementation of this.Quote:
- Do we want Pre and Post filter chains? The Pre filter chain passing the Request along. The Post filter chain passing the Response along.
In post #181 arborint shows it implemented as an array of filters. I believe the filter chain should be implemented as a sort of linked list here, as in a Chain of Responsibility.
I don't see the need for this (based on my experience with a real-world application using InterceptingFilters). Could you give us an example of a filter that needs to know the action that will be executed?Quote:
- Several of the objects may want to know what the "action" is. You should be able to set the action parameter to anything you want (it defaults to 'action'). How is this configurable setting passed to the filters?
Sounds very good, but I think the Action class here should be named something like ActionMapping. It is not the actual Action itself but only an object describing the mapping from the request parameter to an instance of an Action subclass.Quote:
Moving from an array of arrays to an array of objects will allow you to add logic to the actions rather than holding logic in the *Mapper classes I bet. Might be able to get down to just 1 mapper class.PHP Code:$mapper = new ActionLookupMapper('action/', 'home', 'action');
$mapper->addAction(new Action('home', 'home.php', 'home'));
$mapper->addAction(new Action('foo', 'bar.php', 'bar'));
$mapper->addAction(new Action('bar', 'foo.php', 'foo'));
$controller =& new FrontController($mapper);
$controller->execute(new Request());
There's an example of where you should use a template method. Encapsulate all of these little variations in directory and file names in a subclass' template method, like createAction(), called by the parent class. Implementation details like these aren't very relevant to the overall structure design we are trying to do here, IMHO.Quote:
Personally I would drop the prefix / suffix stuff...
I added that because of the wide variablity in how people arrange directories and name files and classes.
Here is an example of the method in which I'd like to see intercepting filters implemented, based on arguments given in my previous post.
All filters and the ActionMapper share a common interface:
Here is the base class for InterceptingFilters and a concrete subclass:PHP Code:/**
* Base interface for InterceptingFilters and ActionMappers
**/
interface RequestProcessor
{
function processRequest(Request $request, Response $response);
}
And here is how you set up the filter chain.PHP Code:/**
* Base class for all kinds of InterceptingFilters
**/
abstract class InterceptingFilter implements RequestProcessor
{
var $next;
function InterceptingFilter(RequestProcessor $next)
{
$this->next = $next;
}
function processRequest(Request $request, Response $response)
{
$this->next->processRequest($request, $response);
}
}
class SomeConcreteFilter extends InterceptingFilter
{
function processRequest(Request $request, Response $response)
{
// carry out pre-tasks
...
// pass on the request to the next filter in the chain
parent::processRequest($request, $response);
// carry out post-tasks
...
}
}
If you guys like this desgin, I'm sure we can even come up with a way to set up the filter chain from a configuration file of some sort (right now it's hard coded in).PHP Code:class FrontController
{
var $filterChain;
function setupChain()
{
$this->filterChain = new SomeConcreteFilter(
new SomeOtherConcreteFilter(
new SomeAuthenticationFilter(
new SomeStatisticsTrackingFilterOrWhatever(
new ActionMapper()
)
)
)
);
}
}
First of all, just wanted to thank everybody for their work in this, I'm looking forwards to working with this.
I'd like to bring up a point about the actiosn though; I personally prefer the rails approach of having related actions be different methods of a single class. e.g. Having a WeblogController class that has index, display, new and create methods. What do you guys think of this approach? I find it helps avoid the explosion of classes you get with the class per action system, and it makes it easy to share helper functions.
CaptainProton: FilterChain could be a separate class with AddFilter() method. It's role is only to hold an array of filters. It's no big difference at all, except that the constructor of your chain breaks up into several AddFilter() calls. And perhaps, later on it could take up another responsibility (maybe filter factory?). Maybe just a matter of taste :).
Do you think lazy-loading would be useful? If there were a large number of items in the chain, or some items are complicated and break down into a multitude of classes, it might be worth looking at.Quote:
Originally Posted by Captain Proton
Failed authentication for example would (I assume) break out of the chain and redirect to another configuration - hence anything following this wouldn't be needed.
I like this also. It really should only be a matter of writing a new type of Mapper I think.Quote:
Originally Posted by 33degrees
- matt
EDIT:
You could always create a parent class full of common functionality, then extend your actions. This actually could make your actions easier to understand if you use parent::someMethod() instead of $this->someMethod() . It does become a little messy/hard to read/understand if you have one class and a bunch of action methods + then helper methods...
Is this not already possible using an ActionMapper? For example (completely untested, as in not even parsed by PHP)Quote:
Originally Posted by 33degrees
Then, within you're WebController.php:PHP Code:<?php
error_reporting(E_ALL);
require_once('skeleton.inc.php');
// ActionMapper with map that defines the file and class for each actions
require_once('ActionMapper.php');
$map = array(
'index' => array(
'file' => 'WeblogController.php',
'class' => 'WeblogController',
),
'display' => array(
'file' => 'WeblogController.php',
'class' => 'WeblogController',
),
'new' => array(
'file' => 'WeblogController.php',
'class' => 'WeblogController',
),
'create' => array(
'file' => 'WeblogController.php',
'class' => 'WeblogController',
),
);
$mapper =& new ActionMapper('action/', 'home', 'action');
$mapper->setMap($map);
$controller =& new FrontController($mapper);
$controller->execute(new Request());
?>
Now, obviously not the cleanest solution, but with a bit of modification (make small modifications to ActionMapper, and maybe the actual FrontController) you can make it more clean.PHP Code:<?php
class WebController {
// Of course, we don't really want to re-define $action_param
// Just have a Config class or something, so we can get the original action param
var $action_param = 'whatever';
function index() {}
function display() {}
function create() {}
function create() {}
function execute($request, $response) {
// Get the action again, we know it is a valid action, because it got this far.
// It has to untainted again, so maybe make that function available to any object
// Unless you want to ActionMapper::untaint($action), which is not reccomended.
$method = $request->get($this->action_param);
// We call the appropiate method.
$this->$method();
}
}
?>
Is there a better way to do this array with the current setup? Re: the Rails request, one of the nice things there is you define a method on the "WeblogController" class, and that's it. No config per action :)Quote:
PHP Code:$map = array(
'index' => array(
'file' => 'WeblogController.php',
'class' => 'WeblogController',
),
'display' => array(
'file' => 'WeblogController.php',
'class' => 'WeblogController',
),
'new' => array(
'file' => 'WeblogController.php',
'class' => 'WeblogController',
),
'create' => array(
'file' => 'WeblogController.php',
'class' => 'WeblogController',
),
);
Douglas
I think I have a similar solution. Although it could have been fit into the current setup (I think), it didn't really make sense too (IMHO). Tell me what you think, it definetly needs improving.Quote:
Originally Posted by DougBTX
Here is the example file:
PHP Code:<?php
include('skeleton.inc.php');
class PageController
{
function index(&$request, &$response) {
$response->setContent('This is index');
}
function news(&$request, &$response) {
$response->setContent('This is news');
}
}
$controller = new ApplicationController('PageController', 'index', 'action');
$controller->execute(new Request());
?>
One pitfall here is, that is_readable() checks on a concrete filename, while include() may include from several locations, depending on the ini-settings.Quote:
Originally Posted by Overunner
You mention the template methods in your following post, but where do you see missing encapsulation ?Quote:
Originally Posted by Captain Proton
A factory would probably be a better solution than a template method, since we may have to instantiate thoose objects otherwhere in our code.
I appreciate using a CoR rather than a stack (two actually - pre and post). There are some problems though, which however could be solved. McGruff already mentioned lazyloading, which isn't as trivial as with a stack. (At least not if the next-in-chain should be passed in the constructor). Setting it up, probably also seems strange and alien to most php-developers, if they haven't had their share of java at a time.Quote:
Originally Posted by Captain Proton
You have a "RequestProcessor (which is a) base interface for InterceptingFilters and ActionMappers". Why not simply let the mappers be InterceptingFilters ? Is there any logical/semantical reason why this wouldn't work ?
Quote:
Originally Posted by DougBTX
Have a look at my post #90.Quote:
Originally Posted by Ryan Wray
Edit:
Or the attached file
It seems like everybody have their own variation over how to map the request to the dispatcher, and even how the dispatcher should be shaped. For this very reason I think it's a really, really good idea to have the requestmapper seperated from the rest of the FrontController, to allow the user to plug-and-play with different implementations. That's what we have been doing so far. (Or at least I think, I have managed to push us in that direction) So it really hurts me to see that you have removed this abstraction, and tossed the requestmapper directly into the frontcontroller.
I really don't mind the multitude of classes, but I recognize that I'm probably a minority here. If you name the classes in a systematic way (for example ModuleAction) it isn't hard to figure out which is which.As suggested, the helpers could be placed in a base-class.Quote:
Originally Posted by 33degrees
You're solution is much more elegant. I was trying to think of a way to migrate the functionality in the FrontController but for the life of me couldn't (I blame it on lack of sleep, couldn't the whole night!:p).Quote:
Originally Posted by kyberfabrikken
Sounds like a kitchen sink to me. If you want to share helper functions, you make one class inherit from the other or you delegate to another object.Quote:
Having a WeblogController class that has index, display, new and create methods..I find it helps avoid the explosion of classes you get with the class per action system
No and at this stage I really don't see why we should care about it. How many filters can you possibly have that would make your app run so slow that it would need lazy loading? This is premature optimalisation, completely unnessary.Quote:
Do you think lazy-loading would be useful?
Yeah, so you have a couple of unused filters down the chain, big deal?Quote:
Failed authentication for example would (I assume) break out of the chain and redirect to another configuration - hence anything following this wouldn't be needed.
That could work. It's just that both an ActionMapper doesn't really 'intercept' anything, since it's always the last object in the filter chain. I think it's cleaner if you separate the two and extract an interface, but it's not really neccessary, no.Quote:
You have a "RequestProcessor (which is a) base interface for InterceptingFilters and ActionMappers". Why not simply let the mappers be InterceptingFilters ? Is there any logical/semantical reason why this wouldn't work ?
Hello,
I find that what your doing looks pretty like Mojavi. I'm posting this message because I though you've might be interested to know that Mojavi has been forked. Agavi[1] is aiming at faster release and more contributors. Would you like to contribute ? They already have all the tools in place for developping (svn+mailing+chan+...) and they're certainly looking for talented people like you ;)
Cheers,
... zimba
[1] http://www.agavi.org
I weren't that explicit, but I pretty much agree. For smaller apps it might help to maintain the overview if thoose actions are bundled together in one file, so I won't discard the idea completely. I just wouldn't use it myself either.Quote:
Originally Posted by Captain Proton
I think you're right (as allways, I might add). I don't know why we began sticking thoose handles in there in the first place. It may be my fault - I think I was the one suggesting it at first. Can't be bothered to look through all the posts to assert that, though.Quote:
Originally Posted by Captain Proton
I see the reason for the abstract seperation, but there hve been some resistance against too many abstract classes/interfaces earlier on, so I suppose it would be a place to cut to the bone.Quote:
Originally Posted by Captain Proton
Zimba ;
I'm not sure how to reply to that. You're right in as much as we are more or less re-inventing the wheel here. But we are doing it for a purpose: By discussing even the smallest nuts and bolts along the way, we allow ourselves to learn from each other. And even people who aren't directly contributing seem to enjoy following the thread from the side. So the purpose is more like a workshop than about reaching the goal.
They are not helpers, they are mediators. You hardly ever need to share them. In Rails, each action is generally 2-5 lines long. If we could get into that range it would be nice. From the code samples above, it looks like we are at 3 lines of config per action in the $map array.Quote:
Originally Posted by Captain Proton
Douglas
I have made some changes to the code to allow for intercepting filters. I ended up adding two abstract classes, despite my attempts at keeping it at one. Theese are RequestProcessor, and RequestMapper. The latter extends from the former. Processing filters should extend RequestProcessor, while mappers should (obviously) extend RequestMapper. I made some minor changes to the FrontController class, but the api didn't change, so the previous examples are running without changes.
I took the liberty to dispose of ActionLookupMapper and I didn't add the ActionPackMapper, which I uploaded a few posts back. Since the user would mostly use just one or two of the possible mappers, I don't think there is any need to keep them with the main source - they can be percieved as add-ons. We might have to re-introduce them again later on, if we begin to go deep into the ApplicationController and the view-rendering engine, but for now I think they are better left out.
Is it possible to setup a wiki for the definitions of all words ?
This thread is also a good example of patterns applied to php. It could be transposed to something more stable, like on wiki.cc
I was mainly telling that because, if I remember well, on the first pages of this thread somebody told that Mojavi and WACT were too coupled to be useful. And because Agavi has just started (one week old or so), you might be able to change what you don't like.Quote:
Originally Posted by kyberfabrikken
Apart from that I also like this thread and it would be sad if it died already (+2000 posts please :-P)
The main point of referral is Fowler, PoEAA. Apart from that, there have been some referrences to GoF, but they are more basic patterns such as ChainOfResponsibility, Singleton etc. Both books are essential, and IMHO no developer should be without them.Quote:
Originally Posted by zimba
We spice that up with some J2EE patterns, such as DispatcherView, Dispatcher and InterceptingFilter, which Fowler mentions, but doesn't go deeper into. Those should be fully documented at http://java.sun.com/blueprints/corej...erns/Patterns/ although the site is a bit technical, compared to the fluent writings of Fowler.