14

I am working on a product in which the responsibility of one of the modules is to parse XML files and dump the required content in a database. Even though the present requirement is only to parse XML files, I want to design my parsing module in a fashion that I can support any kind of files in the future. The reason for this approach is that we are building this product for a specific client but plan to sell it to other clients in the near future. All the systems in the ecosystem for the current client produce and consume XML files but this may not be the case for other clients.

What have I tried so far? (The Present) I have the following design in mind which is based on the Strategy pattern. I have quickly written down the code in eclipse to convey my design so it would be great if other aspects such as proper way of handling exceptions are ignored for now.

Parser : The strategy interface that exposes a parse method.

 public interface Parser<T> {
        public T parse(String inputFile);
    }

*The reason for using a generic parameter is to allow any return type as well as ensure type safety at compile time.

ProductDataXmlParser A concrete class for parsing a product.xml file that contains product related information. (using XMLBeans)

public class ProductDataXmlParser implements Parser<ProductDataTYPE> {

    public ProductDataTYPE parse(String inputFile) {
        ProductDataTYPE productDataDoc = null;
            File inputXMLFile = new File(inputFile);

        try {
            productDataDoc = ProductDataDocument.Factory.parse(inputXMLFile);
        } catch(XmlException e) {
            System.out.println("XmlException while parsing file : "+inputXMLFile);
        } catch(IOException e) { 
                 System.out.println("IOException while parsing file : "+inputXMLFile);
        }
        return productDataDoc.getProductData();
    }
} 

where : ProductDataTYPE and ProductDataDocument are XMlBean POJO classes generated using an xsd and the scomp command.

The future

If I have a product.txt file to be parsed in the future, I can define my own POJO called ProductData that will hold the required contents of the file. I can then create a concrete class called ProductDataFlatFileParser that implements the Parser interface and have the parse method populate the ProductData POJO for me after parsing the file.

Does this design make sense? Are there any obvious flaws in this design? As the design stands, I am allowing the concrete classes to define the algorithm to parse a file and letting the concrete class decide where to populate the data. The design seems to be more dependent on the domain objects rather than the file formats. Is this a bad thing? Any inputs on how I can improve my design will be highly appreciated.

6
  • Should the software not let the caller know what file formats are supported? How does your software know which parser to invoke?
    – tomdemuyt
    Feb 18, 2013 at 16:39
  • You're looking for feedback on your design, not your actual implementation, so this will be migrated to Programmers where it is on topic. Feb 18, 2013 at 17:00
  • @tomdemuyt Think factory pattern ;)
    – CKing
    Feb 18, 2013 at 18:00
  • 2
    @bot The SO user who told you to post this on Code Review was obviously wrong. You could have read the site's FAQ before posting it, "someone told me to do it" isn't really a good reason for you to do anything. No one's playing ping pong with it, someone volunteered their time and tried to figure out a better place for it instead of outright closing it (which would had been a valid option, as it's off topic for Code Review).
    – yannis
    Feb 18, 2013 at 22:53
  • 2
    Please don't crosspost, either. You're making a mess we have to clean up.
    – Ripped Off
    Feb 20, 2013 at 17:01

3 Answers 3

7

I have a couple of concerns:

  1. I would make sure you actually need a generic design before implementing one. Are you sure you're going to need file types other than XML? If not, why code for them? If you eventually need it, you can retrofit your code at that point. It won't take much longer, you will probably have other requirements that will make the code look different than what you're currently proposing, and you will probably never need to write it anyway. As they say, YAGNI (You Ain't Gonna Need It).
  2. If you do actually need a generic design, and you're quite sure of this, then I'd say that Parser<T> is basically sound. I do see two potential problems: (1) it assumes file input -- what if you're trying to parse a JSON stream that you retrieved from an HTTP response, for example? and (2) it doesn't necessarily provide much value except as part of some larger generic framework where you have lots of different types of parsers for lots of different types of data. But I'm not convinced you need any such large generic framework. You just have a very simple, concrete use case right now, as far as I can tell: parse an XML file into a list of ProductDatas.
  3. It's almost never a good idea to swallow exceptions as you are doing in ProductDataXmlParser. I would convert it to some sort of RuntimeException instead.
6
  • 1
    We are building a product that will communicate with a lot of external systems so I guess it would be a good idea to account for any kind of file/input format. Excellent point about the JSON Stream. That is exactly why I had my parse method in the Parser interface take a String parameter instead of a File parameter. I had a small mistake in my ProductDataXmlParser which I have corrected (Need to pass a file to the XmlBean parser). You are also right about swallowing exceptions. I wrote down this code quickly in eclipse to convey my design on stackoverflow through an example ;)
    – CKing
    Feb 18, 2013 at 15:07
  • OK, cool. I guess I'd make the Parser parameter an InputStream instead of a String, is what I'm saying. :) And good to hear about the exception -- I wasn't sure if that was cut 'n' pasted from your actual code or just sample code for StackOverflow.
    – Eric Galluzzo
    Feb 18, 2013 at 15:45
  • 1
    Also, regarding building a product that will communicate with a lot of external systems, I would hesitate to build any generic code without concrete requirements. For example, until you have at least two types of object to parse, or two file formats, that you need, I would not make a generic Parser interface.
    – Eric Galluzzo
    Feb 18, 2013 at 15:48
  • I'll give a thought to what you are saying. I would like to point out that there are 4 different xml files containing 4 different types of data to be parsed. Product data is just one type of data to be consumed by our system/product.
    – CKing
    Feb 19, 2013 at 5:17
  • I have one more question for you. I am not going to use a Context that is a part of the Strategy pattern. Will that be alright? I am also getting rid of the generic parameters and returning Object in the parse method in the Parser interface. This is to avoid classes that use the Parser to be declared with a type parameter.
    – CKing
    Feb 24, 2013 at 7:43
1

Your design is not a best option. By your design, the only way to use it:

ProductDataXMLTYPE parser = new ProductDataXmlParser<ProductDataXMLTYPE>().parse(input); 
ProductDataTextTYPE parser = new ProductDataTextParser<ProductDataTextTYPE >().parse(input);

We can't see too much benefit from the above example. We can't do things like this:

Parser parser = getParser(string parserName);
parser.parse();

You can consider the following two options before seek the generic:

  • 1, Same output after parse

No matter where the data source is from, the product data will be the same format before you save it to database. It's the contract between the client and your dump service. So I assume you have same ProductData as output. You can simply define a interface:

public interface Parser {
    public ProductData parse(String inputFile);
}

Moreover you you define ProductData as interface if you want it more flexible.

If you don't want the Parser to be mixed with the data. You can split it to two interface:

public interface Parser {
     public void parse(String inputFile);
}
public interface Data {
    public ProductData getData();
}

And your parser will look like this:

public class XMLParser implements Parser, Data {} 
public class TextParser implements Parser, Data {}
  • 2, different output after parse

If the ProductData is not similar and you do want to reuse the Parser interface. You can do it this way:

public interface Parser {
   public void parse(String inputFile);
}

class XMLParse implements {
      @Override
      public void parse(String inputFile);

      ProductDataXML getProductData();        
}

class TextParse implements {
      @Override
      public void parse(String inputFile);

      ProductDataText getProductData();        
}
-2

Just in case you'll prefer to use something already available, I've made a java library called JRecordBind that's based on XMLSchema (backed by JAXB).

It was born to consume/produce fixed length files, and, since XMLSchema defines their structure, you can use it with plain JAXB to marshall/unmarshall XML files

1
  • I am looking for a design to implement a generic parser! I don't think you uderstood my question correctly.:)
    – CKing
    Feb 21, 2013 at 5:06

Not the answer you're looking for? Browse other questions tagged or ask your own question.