views:

132

answers:

3

I want to implement an aplication where I have various Objects that can be interpreted as XML Strings. First I thought of making an interface which made each object implement two methods:

public abstract Element toXML();
public abstract void fromXML(Element element);

The first one turns the object information into the an DOM Element an the secondone loads the information to the object from a DOM element. I ended up defining an static String in each subclass holding the TAG of the element, so I decided to turn the interface into an abstract class and give it more functionality:

public abstract class XmlElement implements Serializable {
     protected static Document elementGenerator;
     public String TAG = "undefined";

     static {
         try {
             elementGenerator = DocumentBuilderFactory.newInstance().newDocumentBuilder().newDocument();
        } catch (ParserConfigurationException e) {
            StateController.getInstance().addLog(
                    new Log(Log.Type.ERROR, "Couldn't load XML parser: " + e));
            System.exit(1);
        }
    }

    public abstract Element toXML();
    public abstract void fromXML(Element element);
}

The element generator is used in the toXML method to generate the Elements. The fault of this design that I'm unable to overcome is that the TAG attribute can't be made static as I wish it to be, mostly because I don't want to instantiate and object of each subclass just to know the TAG it uses. Java doesn't allow to override Static attributes or methods, which is the correct way to overcome this?

+5  A: 

You may be better off refactoring your abstract class as follows:

public abstract class XmlElement implements Serializable {
    protected static Document elementGenerator = createElementGenerator();

    protected static Document createElementGenerator() {
        try {
            elementGenerator = documentBuilderFactory.newInstance().newDocumentBuilder().newDocument();
        } catch (ParserConfigurationException e) {
            StateController.getInstance().addLog(new Log(Log.Type.ERROR, "Couldn't load XML parser: " + e));
            System.exit(1);
        }
    }

    public abstract Element toXML();
    public abstract void fromXML(Element element);
    protected abstract String getTag();
}

This forces the subclass to define a static variable itself and return it to the abstract class via the abstract getTag() method. One thing that concerns me is that the elementGenerator is not threadsafe as far as I see it as it is shared among all instances of all subclasses of XmlElement which may be an issue.

There seem to be other design issues with what you are doing since you can only have one parent class, an interface with a utility class performing the heavy lifting may be a better solution. Also, I am not sure how you planned to use the TAG variable.

Gennadiy
I was thinking of using the TAG variable in the following way: Imagine I have a subclass of the XmlElement class called SubElementXml and I want to get its elements from a Document I would use the getElementsByTagName method indicating the TAG as: SubElementXml.TAG (accesing the tag name without creating a SubElementXml object).I agree that there is a threadsafety issue, I could include a synchronized method that returned an Element object.
eliocs
Why not define a different static variable in each subclass? Your extraction statement would look something like this anyway: document.getElementsByTagName(FirstSubElementXml.FIRST_TAG) and document.getElementsByTagName(SecondSubElementXml.SECOND_TAG). Or is your processing more generic than this?
Gennadiy
+3  A: 

My first concern here, actually, is that your elementGenerator, because it's static, will be a singleton, and since it will maintain state during document generation, you will get conflicts between different instances that are trying to use it. Perhaps I'm missing something?

I'm also not sure what the situation is that you're trying to make easier by making the TAG attribute be static. Are you likely to have a bunch of Class objects (which may represent the class of various subclasses) that you want to get the tag type from without needing to instantiate? That seems... weird. Wouldn't you have actual instances of the subclasses (such that you could simply have an abstract getTag() method that each one needs to implement?

JacobM
A: 

How about something like:

public abstract class TaggedXmlElement implements XmlElement {
    private final String tag;

    TaggedXmlElement(String tag) {
        this.tag = tag;
    }

    public String getTag() {
        return tag;
    }
}

public final class DomXmlElement extends TaggedXmlElement {

    private static Map<String, DomXmlElement> CACHE = new ConcurrentHashMap<String, DomXmlElement>();

    private final Document generator;

    private DomXmlElement(String tag) {
        super(tag);
        try {
            generator = documentBuilderFactory.newInstance().newDocumentBuilder().newDocument();
        } catch (ParserConfigurationException e) {
            StateController.getInstance().addLog(new Log(Log.Type.ERROR, "Couldn't load XML parser: " + e));
            System.exit(1);
        }

    }

    public static DomXmlElement getInstance(String tag) {
        if(tag == null) {
            throw new IllegalArgumentException("tag::null");
        }
        if(CACHE.contains(tag)) {
            return CACHE.get(tag);
        }
        DomXmlElement element = new DomXmlElement(tag);
        CACHE.put(tag, element);
        return element;
    }

    // ... other stuff

}
Droo