Patterns for Abstract Rule Processing

[sweatje]

I have an application under way that is starting to emit bad code smells. I have a number of parameters related to each record (dates, foreign key values, text, etc.) which I need to evaluate against a number of business rules under two circumstances:
a) each time the record is updated when it is in it's "pending" state
and b) when the user attemts to "issue" the record

To date, I have a model class that is basically a Row Data Gateway to handle retrieveing and saving the data in the database. In my form processing (controller) I have been adding private methods to process some of these business rule checks (smell #1, code in the wrong location - belongs in the model, not in the controller). Having these methods private also makes them hard to test (smell #2, hard to test).

So what I am trying to dig at is resources or peoples experiences in trying to model reasonably complex validation logic. Some of this logic goes way beyond "this field required" or "match this regex" kind of validation. In some cases the logic needs to look up the prior issued revision of these records and compare fields from that revision as part of the logic, etc.

Any links or references to appropriate patterns would be greatly appreciated.

[dagfinn]

In Domain-Driven Design, Eric Evans discusses the Specification pattern. That's the most relevant I can think of. It may not be available on the Web.

[sweatje]

That is actually the current book I am reading. May have to skip a bit to get there sooner Thanks.

[sweatje]

I think what I have now could be thought of as a Transactional Script (PoEAA pg 110). Here is a snipit of my code:

    public function isValidToIssue() {
        if ($this->data = $this->atmc->getData($this->sk)) {
            $valid = true;
            $valid &= $this->isPending();
            $valid &= $this->requiredFieldsPopulated();
            $valid &= $this->requiredAlloyTmprPopulated();
            return $valid;
        } else {
            $this->log->push('Invalid key send for issuing ATMC', self::LOG_HEADING);
            return false;
        } 

So by the definition: Transactional Script Organizes business logic by procedures where each procedure handles a single request from the presentation.

I have a sincle method isValidToIssue() that calls several private "child" methods to build up the more complex logic.

But, as Fowler warned, as the logic evolves and gets more complex, it is becoming impossible to adequatly test and maintain this kind of code.

[sweatje]

I think the Specification pattern from DDD is pointing me in the right direction. I am heading towards:

class PolicySelector {
  public function getPolicy($policy_name) 
  // getPolicy returns a chain of Specifications
  // created by chaining the and_() methods of concrete Specification classes
}

abstract class Specification {
  protected $andSpec = false;
  abstract function isSatisfiedBy($doc);
  protected function returnValue($doc, $ret) {
    return (is_object($this->andSpec)) 
    ? $this->andSpec->isSatisfiedBy($doc) && $ret
    : $ret;
  }
  public function and_($spec) {
    if (is_object($spec)) {
        $this->andSpec = $spec;
    }
    return $this;
  }
} 

[Version0-00e]

http://radio.weblogs.com/0124037/2004/02/03.html

What I'm reading at the moment, any help?

[sweatje]

That does help confirm my direction. The tweak I made to the pattern was to nest the Specifications in a chain so I could pass around the policy and still test it with any object. This also meant I had to pass in my user notification object at the time I create the policies so I can get the deeply nested messages. All seems to be going okay and much more testable now

[Version0-00e]

Off Topic:

Looking for this pattern has lead me to find some papers on model driven archetecture, and was wondering just how revelent this is to php development? does anyone use this approach(es) in their development.

EDIT
----

Seams there are a few faults with a model driven archetecture,

http://www.agilemodeling.com/essays/mda.htm

Could be interesting reading.

Thanks, and sorry for going off topic sweatje

Glad to hear it

[lastcraft]

Hi...

I think the Specification pattern from DDD is pointing me in the right direction. I am heading towards:

I haven't really used the Specification pattern (except as a much simpler Query object), but I have a feeling that you are moving to something I have found as well. When you create a mechanical system with plug-in Strategies you tend to think of the Strategy/Policy classes as being in the same package. That means they cannot really see unrelated packages unless the mechanics package can to, so things seem entangled...

(MostlyMechanics <--> MyPolicy) --> IWantThis

IWantThis could be a completely separate package of business specific stuff and nothing to do with MostlyMechanics. Yuk.

However if the Policy/Strategy is abstract (interface) then we can do this...

(MostlyMechanics --> Policy) <-- MyPolicy --> IWantThis

This is good ol' dependency inversion. We have managed to get a visibility arrow pointing from right to left in there. In fact what we now have is the MyPolicy can see both packages, but nothing can see it. This is equivalent to saying that it is in a higher layer so I have started calling this policy promotion (TM ). You push the business rules out to a new higher layer.

My worry is that to work well this has to be done whole heartily, otherwise you will get business logic in both the policy packages and the mechanics packages...

(Mechanics --> AllPolicies) <-- MyPolicy --> IWantThis

This is all off the top of my head and experimental of course. I would love to know if anyone has had any similar experiences.

Also, anyone know anything about rules engines? I think you can get Oracle plug-in ones and Java ones (e.g. Jess). I have just about no knowledge of these.

yours, Marcus

[otnemem]

Hi,

Could you post a little bit of code? I find it easier to understand with examples

Thanks!

[sweatje]

I haven't really used the Specification pattern (except as a much simpler Query object), but I have a feeling that you are moving to something I have found as well. When you create a mechanical system with plug-in Strategies you tend to think of the Strategy/Policy classes as being in the same package. That means they cannot really see unrelated packages unless the mechanics package can to, so things seem entangled...

I think you hit the nail on the head with this. In trying to implement the Specification pattern, the specifications looked a lot like my old friend the Strategy pattern, especially since my PolicySelector object is a Factory for policies (and uses private Factory methods to construct specific concrete specification objects to chain together). Combine this with the fact that my "chain" of specifications looks suspiciously like a Composite pattern... I guess this makes Specification sort of a stripped-polka dotted-plaid pattern

It does now serve my needs much better: very flexible, business logic centralized in the model classes, and a much more testable solution

[sweatje]

Still working on unit tests and building up the actual validation logic, but I think by the end of this adventure I may be able to submit this code for some kind of a "most logic in a single line of PHP" award of some sort (for the return statement)

private function updatePolicy() {
    $m = 'Update Validation';
    $ret = new ValidLockSpecification($this->log, $m);
    return $ret->and_(
        $this->fieldEqualsField('POST_sk', 'CUR_SK'
            ,'The web form you submitted does not match the document you are editing,
            do you have two browser editing windows open?', $m)->and_(
        $this->fieldEquals(
            'CUR_PNDG_INDIC','Y'
            ,'This document is no longer pending (has already been issued).',$m)->and_(
        $this->requiredFieldWarn('CUST', $m)->and_(
        $this->fieldMatchWarn('BEGIN', '~^\d{2}/\d{2}/\d{4}$~'
            ,'Begin date format not mm/dd/yyyy', $m)->and_(    
        $this->fieldMatchWarn('END', '~^\d{2}/\d{2}/\d{4}$~'
            ,'End date format not mm/dd/yyyy', $m)->and_(    
        $this->fieldGtWarn('MAX', 0, 'Maximum volume must be specified', $m)->and_(
        $this->fieldGtFieldWarn('MAX', 'MIN', 'Max must be greater than Min volume', $m)
        )))))));
} 

[lastcraft]

Hi...

Could you post a little bit of code?

It's really to do with whole the application is compartmentalised, rather then with specifc classes and the like.

In web_cache.php, etc say we have...

class PageCache {
    function PageCache($policy) { }
}

class CachePolicy { }

class FixedTimeOut extends CachePolicy {
    function FixedTimeOut($max_age) { }
} 

Let's say we have used this module in lot's of sites. Suddenly we want to have the caching adapt to the load on the server. Luckily we have a module to do this too...

class LoadMonitor {
    function getCurrentLoad() { }
} 

We want to write a class called LoadAdaptiveTimeOut that uses the LoadMonitor in it's decision when asking whether a page should be cached. The obvious thought is to put this class into the caching package. This means that the caching package now depends on the load monitoring and you would have to ship them both if you wanted caching. Alternatively you could place it in the load monitoring package, but what has page caching got to do with monitoring?

So we create a third package that can see the other two and place it in there. For this to be easy for others the caching package must document this behaviour, so that the CachingPolicy subclasses become a visible interface. That's what I mean by promoted. Of course unlike Java, C++, etc. we don't split by namespaces and so "package" is a bit arbitrary. If you think of a package as a shippable component though, the problem is a bit clearer.

Now our new higher level package is still very closely tied to the caching one by that subclass. Its not just tied by interface, but by implementation as well. Code changes in the caching module could break our higher level one. To weaken this linkage we should export just the interface. This means that the CachingPolicy class must be gutted of all working code, making it a passive receiver of questions. Once this is done the dependency inversion is complete. You don't have to go that far of course unless you were genuinely shipping a modular library of some sort (e.g.PEAR).

Once the strategy is passive, most of the subclasses within the caching package will probably be trivial. Simplest then is to remove them and have only the interface.

Which reminds me about something in SimpleTest. I always meant to split the UnitTester package into Tester and Reporter and move all of the reporter stuff into it's own group. Never did get around to that.

yours, Marcus

[sleepeasy]

Marcus, what you're describing sounds very similar to the Separated Interface pattern. Am I correct in this assumption, or have I missed something?

[lastcraft]

Hi...

Marcus, what you're describing sounds very similar to the Separated Interface pattern.

I think your right, but I'm not sure . Subtle isn't it?

His example (the data mapper) is kind of upside down compared with mine. He's got the O/R layer treating the domain object as a strategy for writing rows to the database. I hadn't thought of it like that before.

Most of this patterns lark seem to come down to passing one object into another and letting them sort it out between them. If the object passed in is just data then it's a State, if you ask it to run methods then it's a Strategy and if it calls back to the holding object in one of these methods it's a Visitor. Once you get a handle on the passing in part I reckon you just refactor and let others worry about whether it's a pattern or not.

I'm more intuitive than verbal though.

yours, Marcus

[dagfinn]

I think your right, but I'm not sure . Subtle isn't it?

A diagram would probably help. I would do it, but the weekend is beginning to distract me too much...

[sweatje]

A little update on where I arrived. I am pretty happy with the result, but would welcome comments. One "deviation" from the Specification pattern was I wanted not only to have a true/false for if the tested object met the criteria, I also wanted a more detailed message about why the object failed. To this end I passed in a "NotificationStack" which has a push($mesage, $heading) method for each specification to attach details of why a particular policy failed.

Here is the abstract base class for all of my specifications:

/**
 * make sure to always pass in a NotifyStack object to the constructor and store in $this->log
 * @package    proj
 * @subpackage    models
 * @abstract
 */
abstract class Specification {
    protected $andSpec = false;
    protected $log = false;
    /**
     * method to evalute this specification
     *
     * must always end with return $this->returnValue($atmc, $ret);
     * to correctly process and() method handling
     * @param    DataSpace    $test    the WACT dataspace being evaluated
     * @return    Boolean
     * @abstract
     */
    abstract function isSatisfiedBy($test);
    /**
     * decend the and chain if present compounding return values
     * @param    DataSpace    $test    the WACT dataspace being evaluated
     * @param    Boolean        $ret    this specifications evaluation result
     * @return    Boolean
     */
    protected function returnValue($test, $ret) {
        $ret = (preg_match('/warning/i', get_class($this))) ? true : $ret;
        return (is_object($this->andSpec)) 
            ? $this->andSpec->isSatisfiedBy($test) && $ret
            : $ret;
    }
    /**
     * evalute another specification condition as part of this policy
     *
     * the funky name and_ has a trailing _ because otherwise PHP would consider it
     * the 'and' keyword
     * @param    Specification    another specificaiton to be evaluated
     * @return    Specification    a reference to this Specification object
     */
    public function and_($spec) {
        if (is_object($spec)) {
            $this->andSpec = $spec;
        }
        return $this;
    }
} 

This is an example of a concrete implementation of a Specification:

class FieldInArraySpecification extends Specification {
    protected $field;
    protected $set;
    protected $check;
    protected $msg;
    protected $logdesc;
    protected $empty_check;
    function __construct($field, $set, $log, $key=false, $msg=false, $logdesc='Validation Warning') {
        if (!is_string($field)) {
            throw new SpecificationSetupException('invalid field parameter');
        }
        $this->field = $field;
        if (!is_array($set)) {
            throw new SpecificationSetupException('invalid set parameter');
        }
        $this->set = $set;
        if (!(is_object($log) && method_exists($log, 'push'))) {
            throw new SpecificationSetupException('invalid log parameter');
        }
        $this->log = $log;
        $this->logdesc = $logdesc;
        $this->check = ($key) ? 'array_key_exists' : 'in_array';
        $list = '('.implode(',',$set).')';
        $list_msg = (strlen($list)>50) ? 'valid list' : $list;
        $this->msg = ($msg) ? $msg : "Field '$field' not in $list_msg";
        $this->empty_check = preg_match('/orEmpty/i', get_class($this));
    }
    function isSatisfiedBy($test) {
        if ($this->empty_check && !$test->get($this->field))
            return $this->returnValue($test, true);
        $check_funct = $this->check;
        if (!$ret = $check_funct($test->get($this->field), $this->set)) {
            $this->log->push($this->msg, $this->logdesc);
        }
        return $this->returnValue($test, $ret);
    }
}
class FieldInArrayWarningSpecification extends FieldInArraySpecification {}
class FieldInArrayOrEmptySpecification extends FieldInArraySpecification {}
class FieldInArrayOrEmptyWarningSpecification extends FieldInArraySpecification {} 

One thing that I found interesting was that I wanted the same basic code to implement four different behaviors. Rather than duplicating the code into for different classes, each with their own minor tweak, I actually coded checks within the class itself to evalute the instances own class name and alter its behavior depending on that result. In the end, I have three subclasses with no changes at all to inherited methods. What is peoples opinions on that, clever or appalling?

[Version0-00e]

Rather than duplicating the code into for different classes, each with their own minor tweak, I actually coded checks within the class itself to evalute the instances own class name and alter its behavior depending on that result.

Did you use Reflection for this SweatJe?

[sweatje]

Did you use Reflection for this SweatJe?

Nope, at this point I did not see a need for anything more complicated than:

preg_match('/warning/i', get_class($this)) 

The was also assisted by the assumption that the tested object was going to be a WACT dataspace, and therefore support the get() method.

[DougBTX]

One thing that I found interesting was that I wanted the same basic code to implement four different behaviors. ... What is peoples opinions on that, clever or appalling?

Why not just use inheritance?

Douglas

[sweatje]

Why not just use inheritance?

Douglas

I am using inheritance, but I suspect you question is "Why not just use inheritance the normal way?. Consider how it would look for the "or empty" version compared with the normal version (ignoring constuction):

class FieldInArraySpecification extends Specification {
    function isSatisfiedBy($test) {
        $check_funct = $this->check;
        if (!$ret = $check_funct($test->get($this->field), $this->set)) {
            $this->log->push($this->msg, $this->logdesc);
        }
        return $this->returnValue($test, $ret);
    }
}
class FieldInArrayOrEmptySpecification extends Specification {
    function isSatisfiedBy($test) {
        if (!$test->get($this->field))
            return $this->returnValue($test, true);
        $check_funct = $this->check;
        if (!$ret = $check_funct($test->get($this->field), $this->set)) {
            $this->log->push($this->msg, $this->logdesc);
        }
        return $this->returnValue($test, $ret);
    }
} 

the entire block of

        $check_funct = $this->check;
        if (!$ret = $check_funct($test->get($this->field), $this->set)) {
            $this->log->push($this->msg, $this->logdesc);
        }
        return $this->returnValue($test, $ret); 

is repeated for both sections of the code. This contrasts with what I implemented:

class FieldInArraySpecification extends Specification {
    function isSatisfiedBy($test) {
        if (preg_match('/orEmpty/i', get_class($this) && !$test->get($this->field))
            return $this->returnValue($test, true);
        $check_funct = $this->check;
        if (!$ret = $check_funct($test->get($this->field), $this->set)) {
            $this->log->push($this->msg, $this->logdesc);
        }
        return $this->returnValue($test, $ret);
    }
}
class FieldInArrayOrEmptySpecification extends FieldInArraySpecification {} 

Less typing, less duplication of code. More confusing? Not sure.

[DougBTX]

I am using inheritance, but I suspect you question is "Why not just use inheritance the normal way?

The code in the post above is not what I would consider normal

This would be it with inheritance, where that repeated block of code is inherited from the parent:

class FieldInArraySpecification extends Specification { 
    function isSatisfiedBy($test) {
        $check_funct = $this->check;
        if (!$ret = $check_funct($test->get($this->field), $this->set)) {
            $this->log->push($this->msg, $this->logdesc);
        }
        return $this->returnValue($test, $ret);
    }
}
class FieldInArrayOrEmptySpecification extends FieldInArraySpecification {
    function isSatisfiedBy($test) {
        if (!$test->get($this->field))
            return $this->returnValue($test, true);
        return parent::isSatisfiedBy($test);
    }
} 

That would be more "normal" to me. Whether that code belongs in FieldInArraySpecification or not depends on what else you plan to do in extended versions of the class.

Douglas

[sweatje]

Fair enough, that would work for me.

[sweatje]

Looking into this more, seems like I have run into an area that would require Multiple Inheritance to actually work from that pattern. The "standard" interitance worked fine for a single dimension, i.e.

  function meth1() {
    if (guard_condition_1) return2;
    parent::meth1();
  } 

but I have a case where I have two independant dimensions that affect two methods. Say the set of cases is:
fieldInArray
fieldInArrayOrEmpty
fieldInArrayWarning
fieldInArrayOrEmptyWarning

Specification is the base class. "Warning" style specification override the returnVal() method, and could be a new base class for Specification to inherit from. Thus we could have:
abstract Specification
abstract WarningSpecification extends Specification

and this would work for:
[php]
FieldInArray extends Specification {}
FieldInArrayOrEmpty extends FieldInArray {
function isSatisfiedBy() {
if (guard) return one way;
parent::isSatisfiedBy();
}
}[/php

but what would I do here?

FieldInArrayWarn extends WarningSpecification {
  //how do we get FieldInArray::isSatisfiedBy without copying?
}
FieldInArrayOrEmptyWarn extends ??? {
  //how do we get FieldInArray::isSatisfiedBy?
  //how do we get WarningSpecificaiton::returnVal?
} 

Edit:

It just occoured to me as I hit submit, WarningSpecification should not be a subclass at all, warning should be a property of the Specification that could be set for any specification and acted on accordingly by the returnValue() method. Sorry for the noise.

[erdsah88]

do u think specifications will work with "lazy load"