HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Library for parsing strings to java types, generic types and collections/arrays

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
genericarraysjavaparsingforlibrarytypesandcollectionsstrings

Problem

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 setOfIntegers = parser.parse("1,2,3,4", new GenericType>(){});
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> 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>(builder.t

Solution

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.

Context

StackExchange Code Review Q#43823, answer score: 3

Revisions (0)

No revisions yet.