On 10/30/06, Garrett Rooney <[EMAIL PROTECTED]> wrote:
On 10/30/06, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote:

> Revised Extension model that allows extensions to be developed based on a 
wrapper/delegation model that
> is decoupled from the underlying parsing infrastructure.
>
>  * Add ElementWrapper and ExtensibleElementWrapper to the model APIs
>  * Modify ExtensionFactory to return an ElementWrapper for a given Element
>  * Modify FOMBuilder and FOMFactory to use the new ExtensionFactoryMap
>  * Modify the Feed Thread and OpenSearch extensions to use the new model
>  * Add a new extension sample

Nice work James!  A few comments:

> +  void setElementWrapper(Element element, Element wrapper);

Are we sure this should be part of the Factory interface?  It looks
like every place it's used we've already cast to FOMFactory, so could
it just live there?  I'm not sure if a new parser back end would keep
track of element wrappers in the same way.

> +public class InReplyToImpl
> +  extends ElementWrapper
> +  implements InReplyTo {

Does it still make sense to have separate Impl classes and interfaces
for the thread code?

Oh, and one more:

 @SuppressWarnings("unchecked")
 public Element newExtensionElement(
   QName qname,
   OMContainer parent,
   OMXMLParserWrapper parserWrapper) {
   Element element = (parserWrapper == null) ?
     new FOMExtensibleElement(qname, parent, this) :
     new FOMExtensibleElement(qname, parent, this, parserWrapper);
   //return factoriesMap.getElementWrapper(element);
     return element;
 }

Can that commented out line be removed?

-garrett

Reply via email to