Introduction

Disclaimer

These comments are in response to the public review period for the JSR-94 Java Rule Engine API 1.0 specification. Bob McWhirter is the author of this page and the opinions expressed herein are solely his and do not necessarily represent the opinions held by other members or users of the drools community.

Qualifications

Bob McWhirter adapted Forgy's Rete algorithm into the Rete-OO algorithm for working conveniently with object-oriented constructs. While he has no formal training in the field of rules and inference engines, he is the implementor of the drools Java-based rules engine that uses the Rete-OO algorithm.

Since the public review release of the JSR-94 distribution, Bob has been working to make drools compliant. Below are his comments.

General

I am dismayed that the public review distribution does not include source code for the javax.rules.* classes and interfaces nor for the reference implementation. Having the source available may have prevented many of these comments, questions and requests for clarification.

I do commend the JSR expert group for creating and making freely available the TCK. I have thus far not reached a point in development to determine its usefulness.

RuleServiceProvider

What is the purpose of the newInstance method? Why does it specify only a a java.lang.Object and not at least a RuleServiceProvider ?

RuleRuntime and Friends

I would expect the RuleSession-specific constants STATEFUL_SESSION_TYPE and STATELESS_SESSION_TYPE to be members of RuleSession , instead of RuleRuntime .

It seems that the NoSuch...Exception form is used more often than ..NotFoundException in my observation.

RuleSession

The getType() method seems to be redundant when the language itself supports instanceof . A user could simply determine if a session was an instance of javax.rules.StatefulRuleSession or javax.rules.StatelessRuleSession . The method adds nothing to the API.

StatefulRuleSession

addObject(..) and executeRules(..)

Does the specification intend to explicitly address deeper semantics on the addObject(..) , removeObject(..) and updateObject(..) methods? The signatures imply that these methods simply affect a queue and do not actually result in propagation through a Rete network. With drools, an added object is immediate propagated through the Rete network. Since drools uses active fact extraction, the add process may perform deep analysis of objects reachable from the original root object. This analysis could result in reportable errors. The signature for addObject(..) does not support this use-case.

In order to avoid losing reportable errors, the drools binding to JSR-94 would require that addObject(..) simply adds objects to an internal list and does not immediately propagate the objects to the Rete network until executeRules is called. This would map directly to the drool's TransactionalWorkingMemory , but leaves no way of implementing the more dynamic WorkingMemory model.

Clarification on executeRules(..) is desired. Does it simply fire items that have been placed on the agenda during previous calls to addObject(..) or does is this method responsible for propagating all objects through the Rete and then firing agenda items?

Additionally, the addObject(..) and executeRules(..) methods seem to present an asymmetry. From client-code, addObject(..) will not cause any rules to actually fire. When executeRules(..) is called, rule firings may include calls to addObject(..) to assert new facts and inferences. From the documentation of executeRules(..) it seems as though objects added during rule firing should propagate and possibly add more rules to the firing agenda. Sometimes addObject(..) has immediate side-effects (when used within a rule consequence) and sometimes it does not (when used from client code).

getObjects(..) and Handles

These comments could have been avoided if the source for the reference implementation was provided.

Since a StatefulRuleSession must associate a Handle with every new Object that is added, it is conceivable that a hash-based index mapping Handle to Object may be used. This would satisfy getObject( handle ) 's requirements. The getObjects(..) methods return a java.util.List which implies an ordered collection. What ordering semantics are implied, if any? If the order is that in which objects were added using addObject(..) , then a burden is placed upon the implementor to maintain an insertion-order collection in addition to the hash-based index. If no order is implied and no uniqueness constraints are implied, then the weaked java.util.Collection interface should be specified as the return type of the getObjects(..) methods.

If the semantics are insertion-order, then addition performance burden is placed upon the implementor as removing items using removeObject(..) from an ordered list is not efficient.

RuleExecutionSet and Friends

What, exactly, does "RuleExecutionSet" mean? It's a mouthful, and I don't know what "Execution" in the name really adds. Would "RuleBase" or "RuleSet" be sufficient? It'd provide a shorter base to build upon for the admin Provider class and interface names. Building up to RuleExecutionSetUnregisterException gets unweildy.

The admin exceptions seem to be poorly named in terms of verb form. I suspect that the shorter verb forms were used in order to keep the names shorter. I would suggest the following:

  • RuleSetCreationException
  • RuleSetRegistrationException
  • RuleSetUnregistrationException

LocalRuleExecutionSetProvider

Once again, the name is too long.

Would you please clarify what you mean by "AST"? I understand that AST stands for Abstract Syntax Tree. Would you consider drool's Rule to be an AST?

RuleExecutionSetProvider

Once again, the name is too long.

The createRuleExecutionSet(..) method that accepts an org.w3c.dom.Document shows a bias towards XML formats. I do not think that requiring an XML format of a rule-engine is wise. This concern doesn't affect the drools projects, but I still feel that these methods should be allowed to throw an UnsupportedOperationException .