I have been following this thread with interest, and now that the discussion has seemed to have skidded to a halt, I’d like to post some of my observations here. Please correct me if I’m wrong.
Harry started this thread about event handling because he didn’t like the idea of writing long and complicated switch-statements in a single PHP-script to handle all possible events. This, I completely agree with; I don’t like them either.
The solution to Harry’s problem is - as it seems now - a couple of classes that can handle the registering and execution of various events. With these classes, a programmer can define a number of events for some page, after which the code connected to the events will take care of handling them. Bye bye switch-statement… Or not?
In theory, the switch-statement is still there. But instead of using long and boring conditionals, the idea is as follows:
- Build a table of possible execution paths, indexed on a simple name (the event)
- Find out which event was triggered
- Lookup the selected event in the table
- Execute the code specified in the table for that event
This is a sound solution, because it separates the algorithm (event selection) from the task at hand (executing events). This allows the first to be reused and parametrized for other problems.
In short, I think this sums up what has been discussed in this thread.
Now, I have a serious problem with the code presented in this discussion. Simply put, I think there’s too much of it. Don’t get me wrong, I don’t disagree with Martin Fowler at all (I wouldn’t dare :)), but I do think a much simpler implementation is possible, and more importantly, needed. How do you explain to a procedural programmer that he shouldn’t write a large and complex switch-statement to handle events, but should instead use about 4 or 5 classes instead? These classes alone are at least twice as big as the whole switch-statement (which, in the end, implements the exact same functionality)!
All in all, the code in this thread made me think a bit of the template engine discussion we had earlier. Indirection after indirection is introduced, but in the end each indirection - a solution to a partial problem - has little effect on the problem as a whole. The result is that we end up with a large pipeline of code blocks executed one after another, each doing very little.
So what can we do about it? I think that by going back to the problem we’re actually trying to solve we can come up with a much simpler solution that could convince a procedural programmer to use OO. A solution that is implemented with little code, is easy to use and read, and introduces little overhead.
Instead of using a complex switch-statement, we’re going to use an event table. A client calls ‘index.php?page=home’, and we translate this request by looking up ‘home’ in the event table that generates pages. As such, the input-side of the problem is pretty simple: it’s just a string. The output-side is a bit harder: code specific for some event needs to be executed. By using OOP, this can easily be solved: implement a baseclass (e.g. ‘Event’) with a single method (‘execute’). Each event is then a subclass of this class, executing specific code. An event table can thus be implemented as a single array:
$events = array(
'home' => new StaticPage('html/home.html'),
'news' => new NewsPage('news/*.html'),
'feedback' => new StaticPage('html/feedback.html')
);
$event =& $events[$_GET['page']];
$event->execute();
And presto, there’s your event handling system, in just three statements…
Of course, there is a serious problem with this solution. The event table is populated with objects (class instances), of which only one is actually used. Thus, there’s a lot of overhead.
To solve that problem, we can use the Factory Method design pattern. Instead of instantiating an event object, we describe how an event should be created, after which we let a factory create an object for us based on this description:
$events = array(
'home' => array('class' => 'StaticPage', 'parameters' => 'html/home.html'),
'news' => array('class' => 'NewsPage' , 'parameters' => 'news/*.html'),
'feedback' => array('class' => 'StaticPage', 'parameters' => 'html/feedback.html')
);
$factory =& new EventFactory();
$event =& $factory->createEvent($events[$_GET['page']]);
$event->execute();
The factory is just another indirection, but it’s a useful one, because it eliminates the overhead incurred earlier. Now, only a single object is actually created, namely the one that represents the event that will be executed.
There are still various problems with this simple solution. For example, an event class can only accept a single argument in its constructor. Also, errors aren’t handled properly. Personally, I don’t like the idea of specifying the event table in code. Why not write one in a separate file, and let the rest of the code use it automatically? For example:
key | event | parameters
home | StaticPage | page = 'html/home.html'
news | NewsPage | filemask = 'news/*.html'
feedback | StaticPage | page = 'html/feedback.html'
Registering an event is now a simple matter of adding a line to the event table. There’s need to edit any code at all.
Of course, the code I presented here is in no way as powerful as the combined code posted by everybody else in this thread. Although the missing functionality can be added pretty easily without adding too many lines of code, that’s not the point of this post. My point is that I think we should take care not to ‘overdesign’ our solutions. Every problem should be solved in a simply a manner as possible. Adding indirection after indirection may result in a design that looks good on paper, but does it actually work better? I don’t think so. Simple is best, is my opinion…
Vincent
