1
\$\begingroup\$

I have a chunk of code here:

public class ParserFactory
{
    public static ParserClass getParserClass(InputStream is, CustomMap customMap,
                                           DataHandler recordHandler, DataHandler columnHandler) throws JsonProcessingException
    {
        ParserClassType ParserClassType = ParserClassType.getParserClassType(customMap.getParser());
        switch (ParserClassType)
        {
            case SPECIFIC_XML:
                return new SpecificXMLParser(is, customMap.getProperties(), recordHandler, columnHandler);
            case XML:
                return new GenericXMLParser(is, customMap.getProperties(), recordHandler, columnHandler);
            case JSON:
                return new GenericJSONParser(is, customMap.getProperties(), recordHandler, columnHandler);
            case SPECIAL:
                return new SpecialParser(is, customMap.getProperties(), recordHandler, columnHandler);
            case FW:
                return new GenericFixedWidthParser(is, customMap.getProperties(), recordHandler, columnHandler);
            case CSV:
                return new GenericCSVParser(is, customMap.getProperties(), recordHandler, columnHandler);
            default:
                throw new IllegalArgumentException("ParserClassType not found.");
        }
    }

    public static String getFileExtension(CustomMap customMap)
    {
        ParserClassType ParserClassType = ParserClassType.getParserClassType(customMap.getParser());
        switch (ParserClassType)
        {
            case SPECIFIC_XML: 
                return SpecificXMLParser.FILE_TYPE_OUTPUT;
            case XML:
                return GenericXMLParser.FILE_TYPE_OUTPUT;
            case JSON:
                return GenericJSONParser.FILE_TYPE_OUTPUT;
            case SPECIAL:
                return SpecialParser.FILE_TYPE_OUTPUT;
            case FW:
                return GenericFixedWidthParser.FILE_TYPE_OUTPUT;
            case CSV:
                return GenericCSVParser.FILE_TYPE_OUTPUT;
            default:
                throw new IllegalArgumentException("ParserClassType not found.");
        }
    }
}

Now, it's not exactly too important what all these arguments are. In getParserClass, I retrieve an enum from ParserClassType. The switch handles all the possible values. A certain value will return a ParserClass.

The other function, getFileExtension, retrieves a static string from a parser class.

As my project grows, I will have many new parsers that I add. I want to make this as flexible and less-hectic as possible. I don't want to keep adding cases to this function. So.

I was looking into EnumMap. I was thinking of mapping an ENUM to a ParserClass name, so for example "XML" would make to "GenericXMLParser.class", and then I could use a ReflectionUtil to create the class with the arguments necessary or retrieve that string. Therefore, if I have to make any additions, all I need to do is add the ENUM to the enum class and make an addition to the map. But, ReflectionUtils seems to require me to provide the path of the parser. Something like "com.package.service.more.bs.SpecificXMLParser". I would rather not do that...

Any ideas?

\$\endgroup\$
11
  • 2
    \$\begingroup\$ Why not using interfaces instead of reflection? \$\endgroup\$ Aug 16, 2020 at 7:31
  • \$\begingroup\$ How would an interface solve my problem? They all extend from an abstract class (ParserClass); the only reason it isn't an interface is because the class also has a bunch of member variables that are shared across all the children classes. But also, in the second function, each parser has a different file output extension. Not sure how interfaces would help here... shed some light? \$\endgroup\$
    – John Lexus
    Aug 16, 2020 at 7:34
  • 1
    \$\begingroup\$ @JohnLexus You might be interested in looking at the Abstract Factory design pattern and shift the knowledge about the concrete file extensions and corresponding parser class types there. \$\endgroup\$ Aug 16, 2020 at 7:56
  • 3
    \$\begingroup\$ For the context vote closer(s), there is plenty of context to review, this does not look like stub code. \$\endgroup\$
    – konijn
    Aug 16, 2020 at 10:11
  • 1
    \$\begingroup\$ @JohnLexus In our project, we have multiple uses of guice, so for me, the solution I proposed was good. I agree with your point. It doesn't make sense to add this dependency just for this use case. \$\endgroup\$ Aug 17, 2020 at 11:52

1 Answer 1

3
\$\begingroup\$

The only problem I see with this code is, that for extending the enum, you need to change two methods in the code, which spreads out changes instead of keeping them local to a single place.

You could however harness the power of the java enum (which can have fields and methods) to make the changes locally.

Something along the lines of:

enum ParserClassType {
    SPECIFIC_XML(SpecificXMLParser.FILE_TYPE_OUTPUT) {
        public ParserClass createParser(InputStream is, etc, ...) {
            return new SpecificXMLParser(...);
        }
    },
    XML(GenericXMLParser.FILE_TYPE_OUTPUT) {
        public ParserClass createParser(InputStream is, etc, ...) {
            return new GenericXMLParser(...);
        }
    },
    ...

    private final String fileExtension;

    private ParserClassType(String fileExtension) {
        this.fileExtension = fileExtension;
    }

    public String getFileExtension() {
        return fileExtension;
    }

    public abstract ParserClass createParser(InputStream is, etc, ...);
}

public static String getFileExtension(...) {
    ParserClassType parserClassType = ...
    return parserClassType.getFileExtension();
}

public ParserClass getParserClass(...) {
    ParserClassType parserClassType =  ...
    return parserClassType.createParser(...);
}

As you see, when you add a new type, you simply have to extend the enum. All the client code stays unmodified.

Apart from that, please look up the java naming conventions. Variables should start with lower case letters and definitely not shadow their class names.

Furthermore, getParserClass() / ParserClass is badly named, as it clearly involves around a concrete parser / parserInstance, not around a class.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for this. The classes and function names have been modified; actually, this code is nowhere to be found in the actual code. Typing out the class names would reveal more than I would like to about what I'm working on. I'll consider this, though.. \$\endgroup\$
    – John Lexus
    Aug 18, 2020 at 16:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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