You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@abdera.apache.org by Garrett Rooney <ro...@electricjellyfish.net> on 2006/10/30 22:56:41 UTC

Re: svn commit: r469180 - in /incubator/abdera/java/trunk: core/src/main/java/org/apache/abdera/factory/ core/src/main/java/org/apache/abdera/model/ examples/src/main/java/org/apache/abdera/examples/extension/ examples/src/main/resources/META-INF/ ex

On 10/30/06, jmsnell@apache.org <jm...@apache.org> 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?

-garrett

Re: svn commit: r469180 - in /incubator/abdera/java/trunk: core/src/main/java/org/apache/abdera/factory/ core/src/main/java/org/apache/abdera/model/ examples/src/main/java/org/apache/abdera/examples/extension/ examples/src/main/resources/META-INF/ ex

Posted by James M Snell <ja...@gmail.com>.
Possibly yes. I'll take a look

Garrett Rooney wrote:
> [snip]
> 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?
> 
> -garrett
> 

Re: svn commit: r469180 - in /incubator/abdera/java/trunk: core/src/main/java/org/apache/abdera/factory/ core/src/main/java/org/apache/abdera/model/ examples/src/main/java/org/apache/abdera/examples/extension/ examples/src/main/resources/META-INF/ ex

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 10/30/06, Garrett Rooney <ro...@electricjellyfish.net> wrote:
> On 10/30/06, jmsnell@apache.org <jm...@apache.org> 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

Re: svn commit: r469180 - in /incubator/abdera/java/trunk: core/src/main/java/org/apache/abdera/factory/ core/src/main/java/org/apache/abdera/model/ examples/src/main/java/org/apache/abdera/examples/extension/ examples/src/main/resources/META-INF/ ex

Posted by James M Snell <ja...@gmail.com>.
Yeah, I was planning on collapsing those into just impl classes.. just
hadn't gotten around to it yet :-)

Garrett Rooney wrote:
> [snip]
> Does it still make sense to have separate Impl classes and interfaces
> for the thread code?
> 
> -garrett
>