Comments inline.

Scott Tavares [[EMAIL PROTECTED]] wrote:
> Well here it is... what do ya think?
> 
<...snip...>
>
> import org.apache.xerces.parsers.DOMParser;
> import org.apache.xerces.dom.traversal.NodeIteratorImpl;
> import org.apache.xerces.domx.traversal.*;
> import org.apache.xerces.dom.DocumentImpl;
> import org.apache.xerces.dom.NodeImpl;
>
I know Xerces is the preferred parser (because it's Apache and one of the few 
with any kind of XML Schema support), but we should probably stick to generic
interfaces if at all possible.
 
> /**
> * This class implements the DOM XML Paser from
> * the Apache Xerces Project. This class also functions
> * as its name implies... ClassMap factory for OPaL ClassMaps
> * objects.
> */
> public class ClassMapFactory
> {
>  private static ClassMapFactory instance;
> 
I like this; it really should follow a factory pattern. And I agree with jon, 
it should probably be a service.

>
>  private ClassMapFactory(PersistenceBroker persistenceBroker)
>  {
>   this.persistenceBroker = persistenceBroker;
>  }
> 
I don't have an answer to this, but I thought about it a lot when I was
writing my implementation. Which is more OO-pure: having PersistenceBroker
get ClassMaps from the factory and adding them to itself or having the 
factory mutate PersistenceBroker by adding ClassMaps to it?


>  /**
>  * This is the primary function of this class
>  * it takes a string that names the source
>  * XML File parses it and create the ClassMaps
>  * accordingly.
>  *
>  *<p>
>  * FIXME:  Add an overloaded method that
>  *   takes a URI (DynamicURI) as a
>  *   parameter.
>  *<p>
>  * @param xmlFile a string with the name and location of an XML file.
>  *<p>
>  */
>  public void buildClassMaps(String xmlFile)
>   try{
>    parser = new DOMParser();
>
The parser to use should be set in the TurbineResources.properties.

> 
>  /**
>  * Recursively process the node of an XML file
>  * and creates the matching classes and joins them
>  * together.
>  *
>  *@param iterator an NodeIteratorImpl from Xerces
>  *
>  */
>  private void processXMLNodes(NodeIteratorImpl iterator)
>  {
<snip>
> }
> 
This basically does the same thing as my SAX implementation, just in the 
DOM way. I don't have enough expertise in this area to determine which way
is better, although my gut tells me SAX. (Brett, oh resident XML expert, 
please insert comments <here>!)


Overall
=======
+ Factory pattern is good, although should be service-based.
- Less explicit refs to Xerces. Parser determined by config file.
/ DOM vs. SAX, the battle continues.


-- 
Christopher Elkins


------------------------------------------------------------
To subscribe:        [EMAIL PROTECTED]
To unsubscribe:      [EMAIL PROTECTED]
Problems?:           [EMAIL PROTECTED]

Reply via email to