Strategy Design Pattern, Generics and TypeSafety

Question

I want to create the following Strategy Pattern combined with Factory, but I want it to be typesafe. I have done the following till now:

public interface Parser<T> {

    public Collection<T> parse(ResultSet resultSet);

}


public class AParser implements Parser<String> {

    @Override
    public Collection<String> parse(ResultSet resultSet) {
             //perform parsing, get collection
        Collection<String> cl = performParsing(resultSet); //local private method
        return cl;
    }
}

public class ParserFactory {

    public enum ParserType {
        APARSER
    }


    public static <T> Parser<T> createParser(ParserType parserType) {


        Parser<?> parser = null;
        switch (parserType) {
        case APARSER:
            parser = new AParser();
            break;
        }
            //unchecked cast happens here
        return (Parser<T>) parser;
    }
}


public class Context {

      public <T> Collection<T> getResults(String query, ParserType parserType) {
          ResultSet resultSet() = getResultSet(query); //local private method
          Parser p = ParserFactory.createParser(parserType);
          Collection<T> results = p.parse(resultSet)
      }

}

In general whatever I attempt, somewhere I will have an unchecked cast. Anyone have an idea how I can refactor the code to be typesafe?

Checking Effective Java I also stumbled upon this pattern:

public final class ParserFactory {

    private ParserFactory() {

    }

    private static class AParser implements Parser<String> {
        @Override
        public Collection<String> parse(ResultSet resultSet) {
            //...
            return new ArrayList<>();
        }
    }

    public static final Parser<String> APARSER = new AParser();

}

So now I can use as Ingo suggested

public <T> Collection<T> getResults(String query, Parser<T> p)

as

getResults("query", ParserFactory.APARSER);

Or would this be better with enums?

Prat

You can't get this type safe as long as the createParser method (or any other method) is supposed to create T out of thin air (so to speak). (Because this says literally: this method will give you a parser of any type, which can't be true). You must pass some input that has T, only then can the compiler check it. Conventionally, one would pass a Class<T>. – Petroglyph

But in your case, it is maybe possible to have the type argument on the ParserType. – Petroglyph

Sadly, enums and generics don't mix well. – Ranson

@PaulBellora: No argument. But for constant-specific methods and enum methods, generic method definitions are no different than class-associated generic methods. – Buskined

@Buskined What I'm saying is that to avoid any unchecked cast, ParserType.APARSER would need to somehow provide a generic type argument of String. In other words it would ideally be <T> Parser<T> createParser(ParserType<T> parserType). But enums can't be generic, so it's a messy hand-off. – Ranson

Answer 1

I'd simply pass a Parser<T> to the getResults() method and forget about that factory stuff. Look, if you say:

public <T> Parser<T> createParser(ParserType typ) { ... }

you are promising that the method will create a parser of any type the caller wants. This is only possible in a type safe way with parsers that all return an empty collection. Moreover, you can't return a Parser<String> from that function, because String is not the same as any type the caller wanted.

If, however, you write:

  public <T> Collection<T> getResults(String query, Parser<T> parser) {
      ResultSet resultSet = getResultSet(query); //local private method
      Collection<T> results = parser.parse(resultSet);
      return results;
  }

you have exactly what you wanted: the getResult method is independent of how the parser works, and yet it returns a collection of the correct type.

And later, instead of

Collection<String> it = (Collection<String>) getResults("query", APARSER);

you say:

Collection<String> it = getResults("query", new AParser());

This is sound and makes sense.

Petroglyph

That is actually my currently working implementation, I just wanted to apply the factory method for further abstraction more for learning purposes. – Prat

Answer 2

I usually use this format. I know that a lot of people does not like it but no one suggested a better approach so far.

public enum ParserType {
    APARSER(new AParser());

    private Parser parser; // this should be an interface which is implemented by AParser

    private ParseType(Parser parser){
        this.parser = parser;
    }

    public Parser getParserInstance() {
        return parser;
    }

}

You can pass Class objects around if you want a new instance every time:

public enum ParserType {
    APARSER(AParser.class);

    private Class<Parser> parserClass;

    private ParseType(Class<Parser> parserClass){
        this.parserClass = parserClass;
    }

    public Parser createParser() {
        return parserClass.newInstance(); // TODO: handle exceptions here
    }

}

Note: I'm eager to find a better approach so if you have some thoughts please share them in a comment.

Anthropology

+1 for recognizing that AParser is stateless and that, as a consequence, only one instance of it ever need be created. That instance can be freely and safely shared to any client code that wants it. It can absolutely be made an instance field associated with an enum constant. In fact, you can delete your second snippet. Because AParser is stateless, there is never any reason to create more than one instance. – Buskined

Answer 3

I applaud your desire to use the Strategy Pattern; +1 for that. I do think Ingo's comments are spot on.

Just an additional comment (taken from Effective Java, 2nd Ed.):

    switch (parserType) {
        case APARSER:
            parser = new AParser();
            break;
    }

To use Joshua Bloch's words, "this solution appears compact and even elegant." However it may also be fragile and difficult to maintain. In general, you should try not to switch on enum constants because whenever you do so, your code will break whenever you change the enum.

This is exactly the right time to use an abstract method definition in your enum and place the desired code right there with the enum constant. Doing this guarantees that you never forget to add the needed enum associated code to your project and assures that whenever you add to your enum that your project won't break.

In fact, if your goals allow for it, it may even be possible or adviseable to move the entire factory method to your enum and have your enum class implement the Strategy interface.

Buskined

"use an abstract method definition in your enum and place the desired code right there with the enum constant.". That sounds interesting, could you provide a concrete example, since I'm running again into typesafety problems. – Prat

The fact remains that you'll lose generic type information about these parsers by using an enum to represent them. – Ranson

@PaulBellora: You're right, of course. I haven't tried it, but I'm just wondering. Since unbounded wildcards are reifiable, can't the parser type associated with an enum constant's instance field simply be Parser<?> and, if so, would it be possible to capture the concrete generic type for the parser at compile time in an enum method using a helper method for wildcard capture by type inference? I'm just thinking out loud. – Buskined

@Buskined For the first part, yes, ParserType could declare an abstract method that returns a Parser<?>, just like your answer suggests (and yeah, this is definitely the right way to go vs. using a switch). For the second part, no, a generic helper method is only good for treating wildcard captures as type parameters, but there's no way to get a type argument like String back from ? without an unchecked cast. – Ranson

@ChrisGeo: I'd be happy to provide an example, but am a bit pressed for time at the moment. Since you're able to access Effective Java, go to the chapter on "Enums and Annotations". J. Bloch put an example of exactly what I'm talking about. Reading that chapter will make you a lifetime fan of Rich Enum Types. – Buskined