Two factory class to generate different interface instances with same condtion

Question

Java Program

enum FILE_TYPE {
    XML, JSON;
}

interface Parser {
    void parse();
}
class XMLparser implements Parser {
    public void parse() { }
}
class JSONparser implements Parser {
    public void parse() { }
}

interface Mapper {
    void map();
}
class XMLmapper implements Mapper {
    public void map() { }
}
class JSONmapper implements Mapper {
    public void map() { }
}

class ParserFactory {
    public static Parser getInstance(FILE_TYPE fileType) {
        switch(fileType) {
        case XML:
            return new XMLparser();
        case JSON:
            return new JSONparser();
        }
        return null;
    }
}

class MapperFactory {
    public static Mapper getInstance(FILE_TYPE fileType) {
        switch(fileType) {
        case XML:
            return new XMLmapper();
        case JSON:
            return new JSONmapper();
        }
        return null;
    }
}

In above java program both factory method generate different interface instance depends on same condition here, i,e both uses same enum FILE_TYPE.

Is it right to use two factory method for this case? constraint is i can't combine both interface into one.

I am very new to java design, Kindly help me

Your naming is completely contrary to Java convention. Classes are **always** is `PascalCase`; `camelCase` is reserved for variables. `UPPER_SNAKE_CASE` is reserved for compile time constants. Please update your code accordingly. — Boris the Spider, Apr 08 '17 at 08:37
As to the actual question; I would suggest you create an `interface ParserMapper` or similar with `getParser` and `getMapper` methods. Have your factory return a `ParserMapper` - that is unless there is any reason to ever use an `XmlMapper` with a `JsonParser`. — Boris the Spider, Apr 08 '17 at 08:39
@BoristheSpider You are right but the idea is parser alone will be reused by other module so i separated both. — sujin, Apr 08 '17 at 08:42
Then pass `parserMapper.getParser` to that module. What's the problem?! — Boris the Spider, Apr 08 '17 at 08:44
`enum FILE_TYPE` - this name is still wrong. See `getInstance(FILE_TYPE fileType)`. — Boris the Spider, Apr 08 '17 at 08:44
@BoristheSpider Mapper implementation would be very huge so i don't want to provide the implementation to the other module. could you pls provide the sample code, if my understanding was wrong — sujin, Apr 08 '17 at 08:48
@sujin "but the idea is parser alone will be reused by other module " So having 2 distinct interfaces and 2 distinct factories is required. Why gathering things that you want to separate ? — davidxxx, Apr 08 '17 at 09:11
@davidxxx i just take this parser and mapper as an example but in actual production code both are different funtionality. — sujin, Apr 08 '17 at 09:20

score 3 · Accepted Answer · edited May 23 '17 at 12:17

No, your current design is incorrect.

Your code is clearly violating open closed principle i.e., you will end up with switch statements at too many places, which is NOT a good design because if you wanted to add HTML parser (or some other parser later), you need to add the switch statements into two Factory classes.

The solution is you need to use abstract factory pattern as shown below:

interface ContentHandler {
     public void parse();
     public void map();
}


public class XMLContentHandler implements ContentHandler {

    public void parse() { }

    public void map() { }
}


public class JSONContentHandler implements ContentHandler {

    public void parse() { }

    public void map() { }
}

class ContentFactory {
    public static ContentHandler getInstance(FILE_TYPE fileType) {
        switch(fileType) {
        case XML:
            return new XMLContentHandler();
        case JSON:
            return new JSONContentHandler();
        }
        return null;
    }
}

I don't want to share the mapper implementation to the system which reusing the parser.

Parsing and Mapping responsibilities should be handled by their own classes (like XMLParser, XMLMapper, JSonParser, etc..) as shown below, which would then complied to single responsibility principle.

public class XMLContentHandler implements ContentHandler {

        @Inject
        private XMLParser xmlParser;

        @Inject
        private XMLMapper xmlMapper;


        public void parse() { 
            xmlParser.parse();
        }

        public void map() { 
            xmlMapper.map();
        }
}

Thanks for your answer. In this parser will be reused by other modules in the system. mapper implementation would be very huge so i don't want to share the mapper implementation to the system which reusing the parser. — sujin, Apr 08 '17 at 08:50
I suggest inject `XMLMapperImpl` and `XMLParserImpl` to the above `XMLHandler` and the responsibilities are separate then. Similarly for JSon — Vasu, Apr 08 '17 at 08:53
i am very new new object oriented design, could you please give me a small code sample — sujin, Apr 08 '17 at 08:55

Two factory class to generate different interface instances with same condtion

1 Answers1