Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I wrote a framework and I want to hear the thoughts of other on the design patterns it uses. Every method contains three design patterns - adapters(strategy), intercepting filters, and observers.

A normal class/method in a class looks like this:

class Run extends PVStaticObject{

    public static function goForARun($miles) {

        $return ='I ran '.$miles. ' miles today';

        return $return;
    }
}

With the design patterns the class/method looks like this:

class Run extends PVStaticObject{

    public static function goForARun($miles) {

        if (self::_hasAdapter(get_class(), __FUNCTION__))
            return self::_callAdapter(get_class(), __FUNCTION__, $miles);

        $miles = self::_applyFilter(get_class(), __FUNCTION__, $miles, array('event' => 'args'));

        $return ='I ran '.$miles. ' miles today';

        self::_notify(get_class() . '::' . __FUNCTION__, $miles, $return);
        $return = self::_applyFilter(get_class(), __FUNCTION__, $return, array('event' => 'return'));

        return $return;
    }
}

A brieft explanation, the adapter will completely change the method by calling another method in its place. Its a way of changing the functionality of a class without modifying the core functionality.

Filters modify variables in a method by passing them out to another class's method or anonymous function where they are modified and return. Normal execution of the method continues.

Observers do not have a return and are purely call another class or anonymous function. They are for event drivin programming.

Examples of adding adapters, filters and observers are below.

Run::addObserver('Run::goForARun', 'run_observer', function($miles, $return){
    echo PVHtml::div('Running '. $miles. ' has caused you to lose 2 pounds', array('style' => 'margin-top:10px;'));
}, array('type' => 'closure'));

Run::addAdapter('Run','goForARun', function($miles){
    echo PVHtml::p('Because of the weather, you were not able to run '.$miles. ' today');
}, array('type' => 'closure'));

Run::addFilter('Run', 'goForARun', 'run_filter', function($data, $options) {

    $data = PVHtml::strong($data);
    $data = PVHtml::p($data);

    return $data;

}, array('type'=> 'closure', 'event' => 'return'));

So what is thoughts and feedback on the design patterns? They are meant to faciliate aspect oriented and event driven design. They also to replace design patterns like dependency injection easily.

PS: More examples of AOP here: https://github.com/ProdigyView/ProdigyView/tree/master/examples/design

share|improve this question
    
Do not approach this problem from the point of view of the common solution of using DI/IoC/Strategy. This is none of those and does NOT strive to be it. These design patterns are about method manipulation and aspect oriented programming. DI/IoC/Strategy is nice for classes with one or two methods but can be boilerplated and cumbersome as classes become larger. Why rewrite a whole class to alter one method? Also this approach does not require a dependency, dependency can be added as needed meaning they are loosely coupled and easily exchangeable. –  Devin Dixon Feb 5 '12 at 18:37
    
We know that you are doing something different. We just don't think its better. –  Winston Ewert Feb 6 '12 at 15:18
add comment

3 Answers

You should relearn design patterns. Until you do this, your framework is bound to be messed up and poorly architected, if only because of your flawed axioms.

The Adapter Pattern is for taking class A implements InterfaceA and wrapping it inside a class Adapter so that you can use it with the same API as class B implements InterfaceB. It has nothing to do with executing different code or whatever inside another class.

The Strategy Pattern is meant to reduce cyclomatic complexity (basically, lots of nested if statements). It works like this:

interface BasicLogicI {
    public function execute();
}

class GreetingLogic implements BasicLogicI {
    public function execute() { echo "Hi!\n"; }
}

class DismissLogic implements BasicLogicI {
    public function execute() { echo "Bye!\n"; }
}

class Speaker {
    protected $strategy;

    public function __construct($strategy) {
        $this->changeContext($strategy);
    }
    public function changeContext($strategy) {
        unset($this->strategy);
        $this->strategy = $strategy;
    }
    public function speak() {
        $this->strategy->execute();
    }
}

$speaker = new Speaker(new GreetingLogic);
$speaker->speak();
$speaker->changeContext(new DismissLogic);
$speaker->speak();
// Output: Hi!
//         Bye!
share|improve this answer
    
Everyone goes after the adapter pattern. One thing I should say its not exactly an adapter but attempts to a mix between adapter and strategy but more flexible. Strategy has the same issues that DI. The pattern implemented above is not about rewriting an entire class and passing the object, interface or not, which can be more work than its worth. The adapter is about easily writing a single method of class and having it loosely coupled if needed. I also should state that another semi popular framework, Lithium, uses the adapter pattern in a similar way. –  Devin Dixon Feb 4 '12 at 23:04
    
Also, the adapter pattern above only has +1 cylomatic complexity compared to strategy. Rather than seeing this as design patterns already known and written about, see this as a different approach to solving a problem. Also, proof of concept it does work: github.com/ProdigyView/Helium-MongoDB . Adapters work in there. –  Devin Dixon Feb 4 '12 at 23:28
    
Just because you say something is an adapter doesn't make it so. That's my point. –  Theodore R. Smith Feb 4 '12 at 23:40
add comment

Firstly, that is a ridiculous of boilerplate to put into every function. That alone means I'd never have anything to do with that code.

Secondly, I don't see how its a good idea. Basically, all of these constructs allow the modification of the behaviour of the object. My functions won't act like I expect them to because somebody's filtered the input, replaced the function with an adapter, and has dozens of observers hanging off of it.

These techniques can be useful, but I think they generally harm code readability. Introducing a framework that encourages doing it everywhere strikes me a recipe for disaster.

Let's consider the example of not running in the rain, comparing DI and your method:

DI

class SunnyRunner
{
     void run()
     {
           print "Running";
     }
}

class RainyRunner
{
     void run()
     {
           print "Staying out of the rain";
     }
}

if(weather.raining)
     runner = new RainyRunner();
else
     runner = new SunnyRunner();

AOP (yours)

class Runner
{
     void run()
     {
           print "Running";
     }
}

if(weather.raining)
{
     runner.adapt("run", function() {
           print "Staying out of the rain";
     });
}

They are somewhat similiar, but to my mind your method gives me a false impression of what the code does. The DI makes it clear that we split into two different versions of Runner depending. Your code gives me the impression there is only one runner, and then subverts that.

share|improve this answer
    
Consider this when comparing DI to these adapters. 1) DI you are building an object with multiple objects. Here you are just replacing the default functionality of a single method. 2) For larger interfaces, DI maybe require a whole rewrite of an entire class. That can be a lot of code. Back to the answer one, you are just modifying one method. 3) DI requires classes. This can be done with anonymous functions. 4) DI requires dependencies to be marked as public. This can be used with protected and privated. For complex functions, these adapters are more flexible and easier to write than DI. –  Devin Dixon Feb 3 '12 at 23:04
2  
@DevinDixon, I grant that you method is more flexible and in at least some cases easier then DI. My concern is that it also makes it harder to follow your code. Sometimes that'll be worth it, but IMO often not. I do use those technique when they are useful. But to build a framework based on the pervasively applying those techniques seems like a bad idea. –  Winston Ewert Feb 3 '12 at 23:39
    
@DevinDixon how does DI require dependencies to be marked as public? –  vascowhite Sep 5 '12 at 20:45
add comment

I'm not really sure that I agree that what you have presented is a solution for providing AOP in PHP. A true AOP solution would allow you to point-cut at any line of code, not just at the entrance and exit of a method call. Also, I would argue that the amount of boilerplate required is counter-productive to the goal of making code easier to read/maintain by separating cross-cutting concerns from business logic.

In my opinion, what you have here is a generic decorator solution, not AOP.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.