3
\$\begingroup\$

I'm an experienced Java developer, however I don't have experience with reflection or Annotation classes. For fun, I tried to develop a CSV Reader class which can parse each row into a particular type.

Here is my code:

package com.richardrobinson;

import java.io.BufferedReader;
import java.lang.annotation.*;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.IntStream;
import java.util.stream.Stream;

import static java.util.Map.*;

/**
 * This class allows CSV text files to conveniently be parsed into a stream objects of the specified type.
 * <p>
 * By default, CSVReader supports {@code Integer, Double, Character, String,} and {@code Boolean} types. Other types may be added via {@link CSVReader#registerParser(Class, Function)}
 * <p>
 * For example, given a class {@code Foo}:
 * <pre>{@code
 * class Foo {
 *     final Integer i;
 *     final String s;
 *
 *     @CSVConstructor public Foo(Integer i, String s) {
 *         this.i = i;
 *         this.s = s;
 *     }
 * }
 * }</pre>
 *
 * and a {@link BufferedReader} {@code reader} whose contents are
 * <pre>
 * num,str
 * 1;hello
 * 2;world
 * </pre>
 *
 * then the reader may be parsed via
 * <pre>
 * var csv = CSVReader.of(reader, Foo.class)
 *      .ignoringHeader()
 *      .withDelimiter(";")
 * </pre>
 *
 * @param <T> the type of the objects. The class of {@code T} must have a constructor which satisfies the following properties:
 *  <ul>
 *      <li>It is annotated with {@link CSVConstructor}</li>
 *      <li>The number of parameters is no more than the number of fields per CSV line</li>
 *      <li>The types of the parameters must be a supported type.</li>
 *  </ul>
 *
 * @author Richard I. Robinson
 */
public class CSVReader<T> {
    /**
     * An annotation which may be applied to a constructor to indicate that such constructor should be used when being instantiated via {@link CSVReader}
     */
    @Retention(RetentionPolicy.RUNTIME)
    @Target({ElementType.CONSTRUCTOR})
    public @interface CSVConstructor {}

    private final BufferedReader reader;
    private final Class<T> clazz;
    private String delimiter = ",";
    private boolean ignoreHeader = false;

    private static final Map<Class<?>, Function<String, ?>> PARSERS = new HashMap<>(ofEntries(
        entry(Integer.class, Integer::parseInt),
        entry(Double.class, Double::parseDouble),
        entry(Character.class, s -> s.charAt(0)),
        entry(String.class, s -> s),
        entry(Boolean.class, Boolean::parseBoolean)
    ));

    /**
     * Enables support for a type {@code T} for CSVReader instances in addition to the types supported by default
     *
     * @param cls the Class to add support for (for example, {@code Foo.class})
     * @param parser a Function mapping a {@link String} to a {@code T}
     * @param <T> the type corresponding to {@code cls}
     */
    public static <T> void registerParser(Class<T> cls, Function<String, T> parser) {
        PARSERS.put(cls, parser);
    }

    private CSVReader(BufferedReader reader, Class<T> clazz) {
        this.reader = reader;
        this.clazz = clazz;
    }

    /**
     * Creates a new CSVReader instance from the specified {@code reader}, whose lines may be parsed into instances of type {@code clazz}. By default, the delimiter used is {@code ","}, and it is assumed there is no header line. These options may be configured via their respective builder methods.
     *
     * @param reader a {@link BufferedReader} containing {@code n} lines of text, with each line containing {@code m} fields separated by a delimiter.
     * @param clazz the class of the type of object that each row is parsed into. For example, {@code Foo.class}
     * @param <T> the type corresponding to {@code clazz}
     * @return a new CSVReader instance, which may be further configured with the builder options
     * @see #withDelimiter(String)
     * @see #ignoringHeader()
     */
    public static <T> CSVReader<T> of(BufferedReader reader, Class<T> clazz) {
        return new CSVReader<>(reader, clazz);
    }

    /**
     * Sets a custom delimiter to be used
     * @param delimiter the delimiter to use to separate fields of each row
     * @return {@code this} CSVReader with the specified delimiter
     */
    public CSVReader<T> withDelimiter(String delimiter) {
        this.delimiter = delimiter;
        return this;
    }

    /**
     * If a header line is present, this method should be invoked so that this CSVReader ignores the first line
     * @return {@code this} CSVReader with the header line ignored
     */
    public CSVReader<T> ignoringHeader() {
        this.ignoreHeader = true;
        return this;
    }

    /**
     * Maps each line of the reader to a parsed instance of type {@code T}. The number of fields per line must be no less than the number of fields of class {@code T}.
     * @return a Stream of instances of type {@code T} corresponding to each line
     */
    public Stream<T> rows() {
        return reader.lines().skip(ignoreHeader ? 1 : 0).map(this::parseRow);
    }

    @SuppressWarnings("unchecked")
    private T parseRow(String row) {
        final var split = row.split(delimiter);
        final var annotatedCtor = Arrays.stream(clazz.getConstructors())
                .filter(ctor -> ctor.isAnnotationPresent(CSVConstructor.class))
                .findFirst()
                .orElseThrow();

        final var ctorParams = annotatedCtor.getParameterTypes();
        final var args = IntStream.range(0, ctorParams.length)
                .mapToObj(i -> PARSERS.get(ctorParams[i]).apply(split[i]))
                .toArray();

        try {
            return (T) annotatedCtor.newInstance(args);
        } catch (Exception e) {
            e.printStackTrace();
        }

        return null;
    }
}

It works perfectly as designed, however I was wondering if there are any best practices with regards to reflection and annotations which I am not using and should be using, or if there's edge case problems in my code. I'm also super open to advice on the class design overall! Thanks!

For an example of the class usage, check out the JavaDoc comment above the class declaration.

\$\endgroup\$
2
\$\begingroup\$

I have some suggestions for your code.

Use a conventional name for the static factory method name

In my opinion, the of name generally aggregate a given set of data into a container; this can cause confusion in this case. I suggest to rename the method to create or newInstance.

public static <T> CSVReader<T> of(BufferedReader reader, Class<T> clazz) {
   return new CSVReader<>(reader, clazz);
}

Use the given class to cast the return object

Use the java.lang.Class#cast method to cast your object, this will make the annotation useless, since the class knows the type while the static cast doesn’t (type erasure).

Before

return (T) annotatedCtor.newInstance(args);

After

return clazz.cast(args);
| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

First of all, to get that out of the way, I despise var and static imports. For me, it makes the code an unreadeable mess that looks like javascript. You would not be allowed to do that on my team.

Now regarding the concrete code:

  • It is not a CSV reader. For real csv, you'd need a quote character and a way to escape the quote. Usually this is done via doubling the quote character in a quoted sequence. (This is so silly, that csv is a traditional example of how-not-to-design-a-file-format.)
  • Using an annotated constructor is severely limited. The csv-reader should rather expect a class adhering to the java bean standard (i.e. default constructer and getters/setters) to be of practical use. (OK, this invalidates the idea to practice working with annotations, but all of a sudden, you could use it for classes which have not specifically crafted for this csv-reader.) Maybe you could implement alternatives?
  • The delimiter char should be settable.
  • Instead of s -> s, use Function.identity()
  • I would recommend to extract the reflection-based analysis to some preparation stage, so that you don't have to repeat it for every single line in your data file. Take a few measurements, I wager that this is the second slowest part in the program right after file I/O.
  • e.printStackTrace() - come on, there must be a better way to handle errors.
  • Making local variables final serves no purpose at all. We can plainly see that you don't set the values a second time, and even if you did, the reader would not care. Sometimes you need this for use in a lambda or inner class, but usually this is just noise.
| improve this answer | |
\$\endgroup\$

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.