Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am a newcomer to design patterns. I read some articles about the abstract factory pattern, and wrote the following simple example:

public interface ParserFactory {
   List<ITransport> getBusList();
   List<ITransport> getTrainList();
   List<ITransport> getPlaneList();   
}


public abstract class XMLParserFactory implements ParserFactory{ }
public abstract class CSVParserFactory implements ParserFactory{ }

public class DOMTransportParser extends XMLParserFactory{

    public List<ITransport> getBusList(){ return null;  }
    public List<ITransport> getTrainList(){ return null; }
    public List<ITransport> getPlaneList(){ return null; } 
 }

 public class CSVTransportParser extends CSVParserFactory{

    public List<ITransport> getBusList(){   return null; }
    public List<ITransport> getTrainList(){ return null; }
    public List<ITransport> getPlaneList(){ return null; } 
 }

}

Is anything wrong with this code? Can it be improved? Do I understand the abstract factory pattern correctly?

share|improve this question
    
"Abstract Factory" is not a pattern but an abomination. I'd strongly recommend to look at Dependency Injection instead (using Guice, Spring, CDI...), which solves the same problem much more elegant and flexible. –  Landei Feb 5 '13 at 13:19
    
Except an Abstract Factory is a type of Dependency Injection (manual dependency injection). You can use an Abstract Factory to implement DI. It all depends on what you want to do - DI seems like overkill for this example, anyway. Too much abstraction is just as bad as no abstraction at all. –  Yuushi Feb 6 '13 at 0:33
add comment

1 Answer

up vote 2 down vote accepted

This isn't really an Abstract Factory. Going on the theme of what you've got above (although I'm not sure what trains, planes and buses have to do with parsing!), an Abstract Factory would look something like the following:

public interface Parser
{
    List<ITransport> getBusList();
    List<ITransport> getTrainList();
    List<ITransport> getPlaneList();
}

public interface ParserFactory 
{
    Parser getParser();
}

public class XMLParser implements Parser
{
     //Implementation of Constructors and Interface methods from Parser
}

public class CSVParser implements Parser
{
     //Implementation of Constructors and Interface methods from Parser
}

public class XMLParserFactory implements ParserFactory
{
    public Parser getParser()
    {
        return new XMLParser();
    }
}

public class CSVParserFactory implements ParserFactory
{
    public Parser getParser()
    {
        return new CSVParser(); 
    }
}

This is then used in some way like the following:

public class Main
{
    //Note: Using a String is for illustrative purposes only. Ideally
    //it should be using some system or file setting. Half the point
    //of this pattern is that the caller doesn't know what kind of 
    //object they are getting back (which probably doesn't make too much
    //sense for parsers, really).
    public static ParserFactory createParserFactory(String type)
    {
        if(type.equals("XML") {
            return new XMLParserFactory();
        } else {
            return new CSVParserFactory();
        }
    }

    public Main(ParserFactory factory)
    {
        Parser parser = factory.getParser();
        List<ITransport> bus = parser.getBusList();
    }

    public static void main(String[] args)
    {
        new Main(createParserFactory("XML"));
    }

So what's the point of all this (it's quite a lot of code, after all). Well, firstly, it insulates the caller from knowledge of the concrete type. They don't know what kind of Parser they're getting back, the implementation details are hidden from them. Secondly, it localizes all object creation through a single point, which means client code will need very minimal updates when things change. For example, say we want to add a new kind of Parser - let's say a JSONParser - to our code. Then we create a factory and parser like we did previously:

public class JSONParser implements Parser 
{ // Implementation details 
}

public class JSONParserFactory implements ParserFactory
{
    public Parser getParser() { return new JSONParser(); }
}

Then we simply add it to our getParserFactory method:

    public static ParserFactory createParserFactory(String type)
    {
        if(type.equals("XML") {
            return new XMLParserFactory();
        } 
        else if(type.equals("JSON") {
            return new JSONParserFactory();
        } else {
            return new CSVParserFactory();
        }
    }

Note that any client code will be calling createParserFactory() and then getParser from that - so ideally, if they want to switch in a JSONParser, the only thing that needs to change in their code is the call to createParserFactory("JSON").

Hopefully this has cleared things up a little bit (and not confused you too much).

share|improve this answer
    
I almost agree with you. But what about this. I agree in part with intefces ParserFactory and Parser. But as I understand every factory should produce own specific type of product XMLParser on my opinion should be interface because for it exist different implementations (DOM, SAX, StAX) in this case I couldn't create object and my abstract factory become builder. Or instead of XMLParser write DOMTransportParser, SAXTransportParser? –  Ray Feb 5 '13 at 8:47
    
So make an interface for XMLParser then. The idea was mainly to show you the basic structure - you can modify it as you please, either by having another layer underneath your XMLParser that returns a specific type (all still wrapped up in a Parser, however). –  Yuushi Feb 6 '13 at 0:28
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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