Now the idea was that this wouldn't be such a big problem as it was in previous incarnations of my library because now I have separated out all the rendering (some people call these painters hence subject of topic) functionality out into separate objects using composition! I've used this technique a lot and its proven very effective. I'm able to keep the roles of my classes more tightly focused and I can even be swap out renderers. I get low coupling and high cohesion. This is the strategy pattern.
Unfortunately not everything can be done this way and so I've still had to use a fair bit of inline functionality. Here's one example. I've got:
- Entity - Compositions for unique identification, rendering and setting of visual attributes
- Widget extends Entity - Composition for getting input (retriever) and inline functionality for attaching/dispatching events, adding errors, validation shorthand, setting initial value
- Text extends Widget - Inline functionality for input filtering
So next I do this:
- Entity - as before
- Widget extends Entity - as before
- Select extends Widget - inline functionality add options to the select
- separate the filtering I wrote for Text
- into a separate class that can interact with most widget
- use composition to get it into both Select and Text
I even managed to do this:
- Entity
- Widget extends Entity
- Container extends Widget - implementation of composite pattern (577 lines here
) - CheckBox_Container extends Container - a checkbox which you can store other widgets in but they are only processed when the checkbox is checked
Code: Select all
class Container extends Widget {
protected function _hasValue()
{
return false;
}I can see myself needing to do this kind of thing more and more and its beginning to get complicated and difficult to maintain. Actually a lot of the places where I've used the composition and the strategy pattern I'm creating new interfaces between classes that are tricky to work with --- In order to get a Filterer that will work with Text and Select I've got to get a value to Filterer at a time when it should only be available the widget itself. Accommodating new features in this way makes superclasses such as Widget and Container into finely tuned, fragile, pieces of class simply for the benefit of their subclasses and they represent a maintenance problem.
Yesterday I realised I had a bug where a form could be spoofed with certain request variables missing and the validations for those variables wouldn't be run. Those validations include those that enforce the presence of values. This is a pretty serious security issue and I realised there was really no way of fixing it apart from a couple of scarily fundamental changes involving.....decorators.
I'm not a fan of decorators. I've tried to use them three times in the past and each time I've discovered it was simpler to do without them. A lot of books give you examples of decorators that can be written more easily without them, using boolean flags or arrays instead. I understood their value and the problems with inheritance but I told myself this time the strategy would save me. To be fair until recently it has worked very well and I had lots of reason why I didn't want to use decorators:
- Lots of small classes make library harder to understand
- Can't decorate public properties so lots more setters and getters
- My widgets don't need to change at runtime and that seems to be the main point of decorators
- I really dislike those decorator classes full of methods that just chain on to the object they are decorating
- Inferior performance
Right so I'm going to have to use decorators. A forth rewrite is a depressing prospect but thinking about it I'm actually quite excited about how its going to remove all those difficult complications. So...and here is the reason for this post...I thought about a new design involving decorators. Instead of this
I can do something like this
- Entity - Compositions for unique identification, rendering and setting of visual attributes
- Widget extends Entity - Composition for getting input (retriever) and inline functionality for attaching/dispatching events, adding errors, validation shorthand, setting initial value
- Text extends Widget - Inline functionality for input filtering
- Entity - Compositions for unique identification, rendering and setting of visual attributes
- EntityDecorator extends Entity - for decorators
- Filtering extend EntityDecorator
- Events extend EntityDecorator
- Errors extend EntityDecorator
- Retrieving extend EntityDecorator
So I tried a prototype of something really simple. A widget that is decorated to become a proper select field. The prototype still has a separate objects for rendering. The code is quite long but simple:
Code: Select all
<?php
/**
* To ensure consistency between the decorator and decorated
*/
interface Widget_Interface {
function getId();
function setId($newId);
function render();
function getLabel();
function setLabel($newLabel);
function getRenderer();
function setRenderer(Renderer_Interface $newRenderer);
}
class Widget implements Widget_Interface {
private $_id, $_label, $_renderer;
public function __construct($id) {
$this->setId($id);
}
public function getId() {
return $this->_id;
}
public function setId($newId) {
return $this->_id = $newId;
}
public function render() {
return $this->_renderer->render($this->_getTransferForRenderer());
}
public function getLabel() {
return $this->_label;
}
public function setLabel($newLabel) {
return $this->_label = $newLabel;
}
public function getRenderer() {
return $this->_renderer;
}
public function setRenderer(Renderer_Interface $newRenderer) {
return $this->_renderer = $newRenderer;
}
protected function _getTransferForRenderer() {
// send renderer the stuff it needs
$obj = new stdClass();
$obj->id = $this->getId();
$obj->label = $this->getLabel();
return $obj;
}
}
interface Renderer_Interface {
function render(stdClass $transferObj);
}
abstract class Renderer implements Renderer_Interface {
protected $_trans;
// Implementation of template method pattern
final public function render(stdClass $transferObj) {
$this->_trans = $transferObj;
return $this->_renderHead()
. $this->_renderContent()
. $this->_renderFoot();
}
protected function _renderHead() {
$t = $this->_trans;
return '<div class="field"><label for="' . $t->id . '">' . $t->label . '</label>';
}
abstract protected function _renderContent();
protected function _renderFoot() {
return '</div>';
}
}
class Renderer_Select extends Renderer {
protected function _renderContent() {
$t = $this->_trans; // $this->_trans already assigned by Renderer::render()
$out = '<select id="' . $t->id . '" name="' . $t->id . '">';
if (!empty($t->options)) {
foreach ($t->options as $value => $label) {
$out.= '<option value="' . $value . '">' . $label . '</option>';
}
}
return $out . '</select>';
}
}
class Widget_Decorator implements Widget_Interface {
protected $_widget;
public function __construct(Widget_Interface $widget) {
$this->_widget = $widget;
}
public function getId() {
return $this->_widget->{__FUNCTION__};
}
public function setId($newId) {
return $this->_widget->{__FUNCTION__}($newId);
}
public function render() {
return $this->_widget->{__FUNCTION__}();
}
public function getLabel() {
return $this->_widget->{__FUNCTION__};
}
public function setLabel($newLabel) {
return $this->_widget->{__FUNCTION__}($newLabel);
}
public function getRenderer() {
return $this->_widget->{__FUNCTION__};
}
public function setRenderer(Renderer_Interface $newRenderer) {
return $this->_widget->{__FUNCTION__}($newRenderer);
}
}
class Select extends Widget_Decorator {
private $_options;
public function addOption($value, $label) {
$this->_options[$value] = $label;
}
protected function _getTransferForRenderer() {
$obj = $this->_widget->{__FUNCTION__}();
$obj->options = $this->_options;
return $obj;
}
}
class Factory {
public function mkSelect($id, $label) {
$widget = new Select(new Widget($id));
$widget->setLabel($label);
$widget->setRenderer(new Renderer_Select());
return $widget;
}
}
$factory = new Factory();
$widget = $factory->mkSelect('foo', 'Foo');
$widget->addOption('cat', 'cat');
$widget->addOption('dog', 'dog');
// I supposed to produce a select with a label of Foo id of foo and cat and dog as options
echo $widget->render();
// In reality: <div class="field"><label for="foo">Foo</label><select id="foo" name="foo"></select></div>So I put it to you great members of the great devnet forum board...what should I do? Clearly I'm going to have to mix real inheritance with decoration but where? and how? and is there a better solution? Maybe I just want you to ask me lots of difficult questions to get me thinking. Maybe I just want some sympathy.