4
\$\begingroup\$

I've done a library that can parse strings to different java types and to List<...>, Set<...>, Map<...> and arrays of such types. Below are some usage examples, to get a better understanding where to start reviewing:

StringToTypeParser parser = StringToTypeParser.newBuilder().build();

Integer i = parser.parse("1", Integer.class);
Set<Integer> setOfIntegers = parser.parse("1,2,3,4", new GenericType<Set<Integer>>(){});
float[] arrayOfFloats = parser.parse("1.3, .4, 3.56", float[].class); 

This review is a follow up to the following review.

The library is called type-parser and it's main purpose is to be used together with reflection. For example invoking a java.lang.reflect.Method, where argument values are read from an external source (xml file for example) and the argument types are read via reflection. type-parser can then parse the values to correct java types before invoking the Method.

  • The code contains a generic way to create a List<..> of any of the supported java types (See: TypeParsers#forLists()), but the type T is never defined in code. Is this ok, or is there a better solution for this?
  • Is there a way to get rid of these @SuppressWarnings("unchecked") in TypeParsers.java ?
  • Is there a more user friendly way to represent a generic type than the GenericType class (from guava TypeToken).
  • Any other comments?

.

import static com.github.drapostolos.typeparser.TypeParserUtility.*;
import java.lang.reflect.GenericArrayType;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.util.*;
public final class StringToTypeParser {
    private final Map<Type, TypeParser<?>> typeParsers;
    final Splitter splitter;
    final Splitter keyValuePairSplitter;
    final InputPreprocessor inputPreprocessor;;
    public static StringToTypeParserBuilder newBuilder() {
        return new StringToTypeParserBuilder();
    }
    StringToTypeParser(StringToTypeParserBuilder builder) {
        this.typeParsers = Collections.unmodifiableMap(new HashMap<Type, TypeParser<?>>(builder.typeParsers));
        this.splitter = builder.splitter;
        this.keyValuePairSplitter = builder.keyValuePairSplitter;
        this.inputPreprocessor = builder.inputPreprocessor;
    }
    public <T> T parse(String input, Class<T> targetType) {
        if (input == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("input"));
        }
        if (targetType == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("targetType"));
        }
        @SuppressWarnings("unchecked")
        T temp = (T) parseType2(input, targetType);
        return temp;
    }
    public <T> T parse(String input, GenericType<T> genericType) {
        if (input == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("input"));
        }
        if (genericType == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("genericType"));
        }
        @SuppressWarnings("unchecked")
        T temp = (T) parseType2(input, genericType.getType());
        return temp;
    }
    public Object parseType(String input, Type targetType) {
        if (input == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("input"));
        }
        if (targetType == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("targetType"));
        }
        return parseType2(input, targetType);
    }
    private Object parseType2(final String input, Type targetType) {
        String preprocessedInput = preProcessInputString(input, targetType);
        if(preprocessedInput == null){
            if (isPrimitive(targetType)) {
                String message = "'%s' primitive can not be set to null. Input: \"%s\"; Preprocessed input: '%s'";
                throw new IllegalArgumentException(String.format(message, targetType, input, preprocessedInput));
            }
            return null; 
        }
        if(typeParsers.containsKey(targetType)){
            return invokeTypeParser(preprocessedInput, targetType, targetType);
        } 
        if(targetType instanceof ParameterizedType){
            ParameterizedType type = (ParameterizedType) targetType;
            Class<?> rawType = (Class<?>) type.getRawType();
            if(List.class.isAssignableFrom(rawType)){
                return invokeTypeParser(preprocessedInput, TypeParsers.ANY_LIST, targetType);
            }
            if(Set.class.isAssignableFrom(rawType)){
                return invokeTypeParser(preprocessedInput, TypeParsers.ANY_SET, targetType);
            }
            if(Map.class.isAssignableFrom(rawType)){
                return invokeTypeParser(preprocessedInput, TypeParsers.ANY_MAP, targetType);
            }
        }
        if(targetType instanceof Class){
            Class<?> cls = (Class<?>) targetType;
            if(cls.isArray()){
                return invokeTypeParser(preprocessedInput, TypeParsers.ANY_ARRAY, targetType);
            }
            if(containsStaticMethodNamedValueOf(cls)){
                return invokeTypeParser(preprocessedInput, TypeParsers.ANY_CLASS_WITH_STATIC_VALUEOF_METHOD, targetType);
            }
        }
        if(targetType instanceof GenericArrayType){
            return invokeTypeParser(preprocessedInput, TypeParsers.ANY_ARRAY, targetType);
        }
        String message = "There is either no registered 'TypeParser' for that type, or that "
                + "type does not contain the following static factory method: '%s.%s(String)'.";
        message = String.format(message, targetType, STATIC_FACTORY_METHOD_NAME);
        message = makeParseErrorMsg(preprocessedInput, message, targetType);
        throw new IllegalArgumentException(message);
    }
    private String preProcessInputString(String input, Type targetType) {
        try {
            return inputPreprocessor.prepare(input, new InputPreprocessorHelper(targetType));
        } catch (Exception e) {
            String message = "Exception thrown from InputPreprocessor: %s [%s] with message:  "
                    + "%s. See underlying exception for more information.";
            message = String.format(message, 
                    inputPreprocessor, inputPreprocessor.getClass(), e.getMessage());
            message = makeParseErrorMsg(input, message, targetType);
            throw new IllegalArgumentException(message, e);
        }
    }
    private Object invokeTypeParser(String input, Type key, Type targetType) {
        try {
            TypeParser<?> typeParser = typeParsers.get(key);
            ParseHelper parseHelper = new ParseHelper(this, targetType);
            return typeParser.parse(input, parseHelper);
        } catch (NumberFormatException e) {
            String message =  String.format("Number format exception %s.", e.getMessage());
            message = makeParseErrorMsg(input, message, targetType);
            throw new IllegalArgumentException(message, e);
        } catch (RuntimeException e) {
            String message = makeParseErrorMsg(input, e.getMessage(),targetType);
            throw new IllegalArgumentException(message, e);
        }
    }
    private boolean isPrimitive(Type targetType) {
        if(targetType instanceof Class){
            Class<?> c = (Class<?>) targetType;
            return c.isPrimitive();
        }
        return false;
    }
}

.

import static com.github.drapostolos.typeparser.TypeParserUtility.*;
import java.lang.reflect.Type;
import java.util.*;
public final class StringToTypeParserBuilder {
    Map<Type, TypeParser<?>> typeParsers;
    Splitter splitter = defaultSplitter();
    Splitter keyValuePairSplitter = defaultKeyValuePairSplitter();
    InputPreprocessor inputPreprocessor = defaultInputPreprocessor();
    StringToTypeParserBuilder() {
        typeParsers = TypeParsers.copyDefault();
    }
    public StringToTypeParserBuilder unregisterTypeParser(Class<?> targetType){
        if(targetType == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("targetType"));
        }
        typeParsers.remove(targetType);
        return this;
    }
    public <T> StringToTypeParserBuilder unregisterTypeParser(GenericType<T> targetType){
        if(targetType == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("targetType"));
        }
        typeParsers.remove(targetType.getType());
        return this;
    }
    public <T> StringToTypeParserBuilder registerTypeParser(Class<? super T> targetType, TypeParser<T> typeParser){
        if(typeParser == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("typeParser"));
        }
        if(targetType == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("targetType"));
        }
        typeParsers.put(targetType, typeParser);
        return this;
    }
    public <T> StringToTypeParserBuilder registerTypeParser(GenericType<T> targetType, TypeParser<T> typeParser){
        if(typeParser == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("typeParser"));
        }
        if(targetType == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("targetType"));
        }
        typeParsers.put(targetType.getType(), typeParser);
        return this;
    }
    public StringToTypeParserBuilder setSplitter(Splitter splitter){
        if(splitter == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("splitter"));
        }
        this.splitter = splitter;
        return this;
    }
    public StringToTypeParserBuilder setKeyValuePairSplitter(Splitter splitter){
        if(splitter == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("splitter"));
        }
        this.keyValuePairSplitter = splitter;
        return this;
    }
    public StringToTypeParserBuilder setInputPreprocessor(InputPreprocessor inputPreprocessor) {
        if(inputPreprocessor == null) {
            throw new NullPointerException(makeNullArgumentErrorMsg("inputPreprocessor"));
        }
        this.inputPreprocessor = inputPreprocessor;
        return this;
    }
    public StringToTypeParser build(){
        return new StringToTypeParser(this);
    }
}

.

public interface TypeParser<T>{
    T parse(String input, ParseHelper helper);
}

.

import java.io.File;
import java.lang.reflect.Array;
import java.lang.reflect.Type;
import java.math.BigDecimal;
import java.util.*;
final class TypeParsers {
    static final Type ANY_LIST = new GenericType<List<?>>() {}.getType();
    static final Type ANY_SET = new GenericType<Set<?>>() {}.getType();
    static final Type ANY_MAP = new GenericType<Map<?, ?>>() {}.getType();
    static final Type ANY_CLASS_WITH_STATIC_VALUEOF_METHOD = ValueOfTypeParser.class;
    static final Type ANY_ARRAY = Array.class;
    static final Type CLASS_TYPE = new GenericType<Class<?>>(){}.getType();
    private static final Type ARRAY_OF_CLASS = new GenericType<Class<?>[]>(){}.getType();
    private static final String BOOLEAN_ERROR_MESSAGE = "\"%s\" is not parsable to a Boolean.";
    private static final String CLASS_ERROR_MESSAGE = "\"%s\" is not parsable to a Class object.";
    private static final String CHARACTER_ERROR_MESSAGE = "\"%s\" must only contain a single character.";
    private static final Map<Type, TypeParser<?>> DEFAULT_TYPE_PARSERS = new HashMap<Type, TypeParser<?>>();
    private static final Map<Class<?>, Class<?>> WRAPPER_TO_PRIMITIVE = new HashMap<Class<?>, Class<?>>();
    private TypeParsers() { throw new AssertionError("Not meant for instantiation"); }
    static Map<Type, TypeParser<?>> copyDefault() {
        return new HashMap<Type, TypeParser<?>>(DEFAULT_TYPE_PARSERS);
    }
    static{
        WRAPPER_TO_PRIMITIVE.put(Boolean.class, boolean.class);
        WRAPPER_TO_PRIMITIVE.put(Byte.class, byte.class);
        WRAPPER_TO_PRIMITIVE.put(Short.class, short.class);
        WRAPPER_TO_PRIMITIVE.put(Character.class, char.class);
        WRAPPER_TO_PRIMITIVE.put(Integer.class, int.class);
        WRAPPER_TO_PRIMITIVE.put(Long.class, long.class);
        WRAPPER_TO_PRIMITIVE.put(Float.class, float.class);
        WRAPPER_TO_PRIMITIVE.put(Double.class, double.class);
    }
    static{
        registerTypeParser(ANY_LIST, TypeParsers.forLists());
        registerTypeParser(ANY_SET, TypeParsers.forSets());
        registerTypeParser(ANY_MAP, TypeParsers.forMaps());
        registerTypeParser(ANY_ARRAY, TypeParsers.forArrays());
        registerTypeParser(ANY_CLASS_WITH_STATIC_VALUEOF_METHOD, new ValueOfTypeParser());
        registerTypeParser(Boolean.class, new TypeParser<Boolean>(){
            @Override
            public Boolean parse(final String value0, ParseHelper helper) {
                String value = value0.trim().toLowerCase();
                if(value.equals("true")){
                    return Boolean.TRUE;
                } else if(value.equals("false")){
                    return Boolean.FALSE;
                }
                throw new IllegalArgumentException(String.format(BOOLEAN_ERROR_MESSAGE, value0));
            }
        });
        registerTypeParser(Character.class, new TypeParser<Character>() {
            @Override
            public Character parse(String value, ParseHelper helper) {
                if(value.length() == 1){
                    return Character.valueOf(value.charAt(0));
                }
                throw new IllegalArgumentException(String.format(CHARACTER_ERROR_MESSAGE, value));
            }
        });
        registerTypeParser(BigDecimal.class, new TypeParser<BigDecimal>() {
            @Override
            public BigDecimal parse(String value, ParseHelper helper) {
                try {
                    return new BigDecimal(value.trim());
                } catch (NumberFormatException e){
                    String message = String.format("For input string: \"%s\"", value.trim());
                    throw new NumberFormatException(message);
                }
            }
        });
        registerTypeParser(Byte.class, new TypeParser<Byte>() {
            @Override
            public Byte parse(String value, ParseHelper helper) {
                return Byte.valueOf(value.trim());
            }
        });
        registerTypeParser(Integer.class, new TypeParser<Integer>() {
            @Override
            public Integer parse(String value, ParseHelper helper) {
                return Integer.valueOf(value.trim());
            }
        });
        registerTypeParser(Long.class, new TypeParser<Long>() {
            @Override
            public Long parse(String value, ParseHelper helper) {
                return Long.valueOf(value.trim());
            }
        });
        registerTypeParser(Short.class, new TypeParser<Short>() {
            @Override
            public Short parse(String value, ParseHelper helper) {
                return Short.valueOf(value.trim());
            }
        });
        registerTypeParser(Float.class, new TypeParser<Float>() {
            @Override
            public Float parse(String value, ParseHelper helper) {
                return Float.valueOf(value);
            }
        });
        registerTypeParser(Double.class, new TypeParser<Double>() {
            @Override
            public Double parse(String value, ParseHelper helper) {
                return Double.valueOf(value);
            }
        });
        registerTypeParser(File.class, new TypeParser<File>() {
            @Override
            public File parse(String value, ParseHelper helper) {
                return new File(value.trim());
            }
        });
        registerTypeParser(String.class, new TypeParser<String>() {
            @Override
            public String parse(String value, ParseHelper helper) {
                return value;
            }
        });
        registerTypeParser(Class.class, new TypeParser<Class<?>>() {

            @Override
            public Class<?> parse(String input, ParseHelper helper) {
                try {
                    return Class.forName(input.trim());
                } catch (ClassNotFoundException e) {
                    throw new IllegalArgumentException(String.format(CLASS_ERROR_MESSAGE, input));
                }
            }
        });
        registerTypeParser(ARRAY_OF_CLASS, new TypeParser<Class<?>[]>() {
            @Override
            public Class<?>[] parse(String input, ParseHelper helper) {
                List<String> strList = helper.split(input);
                Class<?>[] array = new Class<?>[strList.size()];
                for(int i = 0; i < strList.size(); i++){
                    Class<?> element = helper.parse(strList.get(i), Class.class);
                    array[i] = element;
                }
                return array;
            }
        });
    }
    private static void registerTypeParser(Type type, TypeParser<?> typeParser) {
        DEFAULT_TYPE_PARSERS.put(type, typeParser);
        if(WRAPPER_TO_PRIMITIVE.containsKey(type)){
            Class<?> primitiveType = WRAPPER_TO_PRIMITIVE.get(type);
            DEFAULT_TYPE_PARSERS.put(primitiveType, typeParser);
        }
        if(type.equals(Class.class)){
            DEFAULT_TYPE_PARSERS.put(CLASS_TYPE, typeParser);
        }
    }
    private static <T> TypeParser<T> forArrays(){
        return new TypeParser<T>() {
            @Override
            public T parse(String input, ParseHelper helper) {
                List<String> strList = helper.split(input);
                Class<?> componentType = helper.getComponentClass();
                Object array = Array.newInstance(componentType, strList.size());
                for(int i = 0; i < strList.size(); i++){
                    Object element = helper.parse(strList.get(i), componentType);
                    Array.set(array, i, element);
                }
                @SuppressWarnings("unchecked")
                T temp = (T) array;
                return temp;
            }
        };
    }
    private static <T> TypeParser<List<T>> forLists() {
        return new TypeParser<List<T>>() {
            public List<T> parse(String input, ParseHelper helper) {
                Class<T> targetType = getParameterizedTypeArgument(helper);
                List<T> list = new ArrayList<T>();
                for(String value : helper.split(input)){
                    list.add(helper.parse(value, targetType));
                }
                return list;
            }
        };
    }
    private static <T> TypeParser<Set<T>> forSets() {
        return new TypeParser<Set<T>>() {
            public Set<T> parse(String input, ParseHelper helper) {
                Class<T> targetType = getParameterizedTypeArgument(helper);
                Set<T> set = new LinkedHashSet<T>();
                for(String value : helper.split(input)){
                    set.add(helper.parse(value, targetType));
                }
                return set;
            }
        };
    }
    private static <K,V> TypeParser<Map<K, V>> forMaps() {
        return new TypeParser<Map<K, V>>() {
            private static final int KEY = 0;
            private static final int VALUE = 1;
            public Map<K, V> parse(String input, ParseHelper helper) {
                Class<K> keyType = getParameterizedTypeArgument(helper, KEY);
                Class<V> valueType = getParameterizedTypeArgument(helper, VALUE);
                Map<K, V> map = new HashMap<K, V>();
                for(String entryString : helper.split(input)){
                    List<String> entry = helper.splitKeyValuePair(entryString);
                    map.put(helper.parse(entry.get(KEY), keyType), helper.parse(entry.get(VALUE), valueType));
                }
                return map;
            }
        };
    }
    private static <T> Class<T> getParameterizedTypeArgument(ParseHelper helper) {
        return getParameterizedTypeArgument(helper, 0);
    }
    private static <T> Class<T> getParameterizedTypeArgument(ParseHelper helper, int index) {
        Class<?> type = helper.getParameterizedTypeArguments().get(index);
        @SuppressWarnings("unchecked")
        Class<T> temp = (Class<T>) type;
        return temp;
    }
}

.

import static com.github.drapostolos.typeparser.TypeParserUtility.*;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;

final class ValueOfTypeParser implements TypeParser<Object>{
    private static final Object STATIC_METHOD = null;
    public Object parse(String input, ParseHelper helper) {
        Class<?> targetType = (Class<?>) helper.getTargetType();
        Method method = getMethodNamedValueOf(targetType);
        try {
            if(targetType.isEnum()){
                input = input.trim();
            }
            method.setAccessible(true);
            return method.invoke(STATIC_METHOD, input);
        } catch (InvocationTargetException e) {
            // filter out the InvocationTargetException stacktrace/message.
            throw new IllegalArgumentException(makeErrorMsg(input, targetType), e.getCause());
        } catch (Throwable t) {
            throw new IllegalArgumentException(makeErrorMsg(input, targetType), t);
        }
    }
    private String makeErrorMsg(String input, Class<?> targetType) {
        String methodSignature = String.format("%s.%s('%s')", targetType.getName(), STATIC_FACTORY_METHOD_NAME, input);
        String message = " Exception thrown in static factory method '%s'. "
                + "See underlying exception for additional information.";
        message = String.format(message, methodSignature);
        return makeParseErrorMsg(input, message, targetType);
    }
}

.

import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.Type;
final class TypeParserUtility {
    static final String STATIC_FACTORY_METHOD_NAME = "valueOf";
    private static final Splitter DEFAULT_SPLITTER = new DefaultSplitter(); 
    private static final Splitter DEFAULT_KEY_VALUE_PAIR_SPLITTER = new KeyValuePairSplitter();
    private static final InputPreprocessor DEFAULT_INPUT_PREPROCESSOR = new NullInputPreprocessor();
    private TypeParserUtility() { throw new AssertionError("Not meant for instantiation"); }
    static InputPreprocessor defaultInputPreprocessor(){
        return DEFAULT_INPUT_PREPROCESSOR;
    }
    static Splitter defaultSplitter(){
        return DEFAULT_SPLITTER;
    }
    static Splitter defaultKeyValuePairSplitter(){
        return DEFAULT_KEY_VALUE_PAIR_SPLITTER;
    }
    static String makeNullArgumentErrorMsg(String argName) {
        return String.format("Argument named '%s' is illegally set to null!", argName);
    }
    static String makeParseErrorMsg(String input, String message, Type targetType) {
        return String.format("Can not parse \"%s\" to type \"%s\" [instance of: %s] due to: %s", 
                input, targetType, targetType.getClass().getName(), message);
    }
    static boolean containsStaticMethodNamedValueOf(Class<?> type){
        Method method = getMethodNamedValueOf(type);
        if(method == null){
            return false;
        }
        if (!Modifier.isStatic(method.getModifiers())) {
            return false;
        }
        return true;
    }
    static Method getMethodNamedValueOf(Class<?> targetType){
        try {
            return targetType.getDeclaredMethod(STATIC_FACTORY_METHOD_NAME, String.class);
        } catch (Exception e) {
            return null;
        }
    }
}

.

public interface Splitter {
    List<String> split(String input, SplitHelper helper);
}

.

final class DefaultSplitter implements Splitter{
    @Override
    public List<String> split(String input, SplitHelper helper) {
        if(input.trim().isEmpty()){
            return new ArrayList<String>();
        }
        return Arrays.asList(input.split(","));
    }
}

.

final class KeyValuePairSplitter implements Splitter{
    @Override
    public List<String> split(String input, SplitHelper helper) {
        return Arrays.asList(input.split("="));
    }
}

.

import java.lang.reflect.Type;
import java.util.List;
public final class SplitHelper {
    private static final SplitHelper IGNORED = null;
    private final Type targetType;
    SplitHelper(Type targetType) {
        this.targetType = targetType;
    }
    public List<String> splitWithDefaultSplitter(String input){
        return TypeParserUtility.defaultSplitter().split(input, IGNORED);
    }
    public Type getTargetType() {
        return targetType;
    }
}

.

import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
public abstract class GenericType<T> {
    private final Type type;
    public GenericType() {
        if(GenericType.class != getClass().getSuperclass()){
            String errorMsg = "'%s' must be a direct subclass of '%s'";
            errorMsg = String.format(errorMsg, getClass().getName(), GenericType.class.getName());
            throw new IllegalArgumentException(errorMsg);
        }
        Type t = getClass().getGenericSuperclass();
        if(t instanceof ParameterizedType){
            ParameterizedType superClass = (ParameterizedType) t;
            type = superClass.getActualTypeArguments()[0];
        } else {
            String message = "'%s' must be parameterized (for example \"new GenericType<List<Integer>>(){}\"), "
                    + "it can not be of raw type \"new GenericType(){}\".";
            throw new IllegalStateException(String.format(message, getClass().getName()));
        }
    }
    final Type getType(){
        return type;
    }
    @Override
    final public String toString() {
        return type.toString();
    }
}

.

public interface InputPreprocessor {
    String prepare(String input, InputPreprocessorHelper helper);
}

.

import java.lang.reflect.Type;
public final class InputPreprocessorHelper {
    private final Type targetType;
    InputPreprocessorHelper(Type targetType) {
        this.targetType = targetType;
    }
    public Type getTargetType(){
        return targetType;
    }
    public String prepareWithDefaultInputPreprocessor(String input) {
        return TypeParserUtility.defaultInputPreprocessor().prepare(input, this);
    }
}

.

final class NullInputPreprocessor implements InputPreprocessor{
    @Override
    public String prepare(String input, InputPreprocessorHelper helper) {
        if (input.trim().equalsIgnoreCase("null")) {
            return null;
        }
        return input;
    }
}

Javadoc has been removed for brevity. The full code (w/ javadoc/unit tests) can be found here: https://github.com/drapostolos/type-parser

\$\endgroup\$
  • \$\begingroup\$ There is a fair amount to go through here... it is on my radar.... but will probably be a day before I get to it. \$\endgroup\$ – rolfl Mar 10 '14 at 13:33
3
\$\begingroup\$

There is a lot of detail in your code, and there is no way to just scan it, and look for problems/patterns, etc.

Still, here are some observations for the moment, and, depending on time availablility, I may come back with some additional comments (maybe posted as a different answer, we will see).

General:

As a general note, you are reinventing some (really big) wheels here... there are a number of formats/transformations available to communicate/save Java objects: XML (multiple systems), SQL(multiple systems), JSON(multiple systems), plain text(Multiple systems), Beans, Serialization, Externalizable, Parcels, etc. Many of these mechanisms have Annotation-style markup to make the process easier (hibernate, etc.). There are a lot of lessons you can learn from other implementations, and one of those lessons may just be: "Oh, this works for me, I don't need to build my own..."

I am somewhat surprised that you feel there is a need for another mechanism....

Conventions:

  • StringToTypeParser.newBuilder() is the entry point to your library.... but, why does it return StringToTypeParserBuilder and not StringToTypeParser ? Your newBuilder() method is on the wrong class... it should be on StringToTypeParserBuilder.newBuilder(). This makes the builder your 'core' class.

    I am not sure if that is what you want, but, as it stands at the moment, it is inconsistent with standards.

  • The public interface TypeParser class does not need to be public. I believe all the implementations of it are package-private, and this interface can be as well.

  • When de-serializing a Set, you use a LinkedHashSet for the actual implementation. I can understand why you may want to preserve the set 'order'. But, on the Map side, you use a regular HashMap.... why not a LinkedHashMap?

Potential bugs.

  • Your DefaultSplitter does not do any form of , (comma) escaping on the split. If an input String has a comma in the value, the split will fail.

  • The KeyValue splitter should use a split-limit of 2 so that the value can contain an = (equals) character: return Arrays.asList(input.split("=", 2));

  • In your getMethodNamedValueOf() method you use the Java reflection method return targetType.getDeclaredMethod(STATIC_FACTORY_METHOD_NAME, String.class);. This will nly work if the actual class declares the valueOf() method. If the class has an inheritance chain, and an ancestor in the chain actually declares the method, then this call will return null, even though the class has the method.

    It is a tricky thing to get reflection right.... you probably want to be using getMethod(...), because that will walk the inheritance for you, but, unfortunately, the methods all need to be public for that.... ;-)

Generics unchecked warnings

As far as I can tell, the generics are mostly OK. You can reduce the number of unchecked annotations you have by declaring it for the whole class ;-) But, you only have two, and, for a reflective system, that is not too bad.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your comments! I know there are other libraries that do similar things, but they also do A LOT of other things too, where the parsing mechanism is just a small fraction of the offered features. I'm trying to avoid adding a 500K size dependencies, just for parsing/converting a string to a java type. I usually end up with writing a long if/switch statement (+ more code if generic types are to be parsed too). I want to avoid copy/paste code between libraries by making a lightweight library doing only the parsing stuff. \$\endgroup\$ – drapostolos Mar 11 '14 at 20:44
  • \$\begingroup\$ I don't think the "newBuilder()" method goes against any standard. It is very similar to how "Effective Java, 2nd edition" describes the builder pattern: "new StringToTypeParser.Builder()", but with the Builder class separated from the Built class (and no use of public constructors). The TypeParser interface needs to be public, as the user can register his/her own TypeParser implementations. Example: StringToTypeParser parser = StringToTypeParser.newBuilder().registerTypeParser(MyClass.class, new MyTypeParser()).setSplitter(new MySplitter()).setInputPreprocessor(...).build(); \$\endgroup\$ – drapostolos Mar 11 '14 at 20:59
  • \$\begingroup\$ @drapostolos - OK with the public TypeParser interface then. About the newBuilder(). I am not worried about the concept of the method, but I am concerned as to why it is on the class StringToTypeParser instead of StringToTypeParserBuilder. \$\endgroup\$ – rolfl Mar 11 '14 at 21:01
  • \$\begingroup\$ Will change to LinkedHashSet, and add a limit to 2 in the KeyValue splitter. I missed that :) Will also look into the geetDeclaredMethod vs. getMethod, I was not aware of the differences! You're also right about the "no escaping" of ",", will look into that as well. \$\endgroup\$ – drapostolos Mar 11 '14 at 21:04
  • \$\begingroup\$ Regarding the newBuilder(). One more reason to place it in the StringToTypeParser class is that the user only needs to care about one class, due to the chaining possibility. Only one entry class :) StringToTypeParserBuilder.newBuilder() would imply to return a class to build the Builder class. \$\endgroup\$ – drapostolos Mar 11 '14 at 21:12

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.