2
\$\begingroup\$

On my job I had the task to process multiple excel sheets, so I tried to make a generic component to make the conversion between the cell values and a java object type.

The first thing that came to mind was a generic interface that will handle the processing of a particular column:

public interface ExcelColumnProcessor<T> {

    T process(Cell cell);

}

So, for example, I can have:

public class SimpleDateProcessor implements ExcelColumnProcessor<Date> {

    @Override
    public Date process(Cell cell) {
        return DateUtils.format(DateUtils.DateFormat.DD_slash_MM_slash_YYYY, cell.getStringCellValue().trim());
    }

}

which whill handle the processing of values like '01/01/2015', and so forth.

The next problem was, how to tie the column with the processor. For this I made an annotation:

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.FIELD)
public @interface ProcessWith {

    long columnIndex();
    Class<? extends ExcelColumnProcessor<?>> processorClass();

}

The idea was that this annotation could be use like the following:

public class MyExcelRow {

    @ProcessWith(columnIndex = 1, processorClass = SimpleStringProcessor.class)
    private String someField;

    @ProcessWith(columnIndex = 2, processorClass = SimpleDateProcessor.class)
    private Date someDate;

}

The class MyExcelRow would represent a row of a particular excel sheet, and some of it's fields (could be all or not, it's just a pojo) represent the column values in that particular row.

Now, I needed something to handle the processing itself, to tie everything together, so I made this:

public final class Processor<T> {

    private Map<Long, MetaData> processors = new ConcurrentHashMap<>();

    private Processor(Map<Long, MetaData> processors) {
        this.processors = processors;
    }

    private void checkObjectIsNotNull(T objectToProcess) {
        if (objectToProcess == null) {
            throw new IllegalArgumentException("The object cannot be null");
        }
    }

    public void process(T objectToProcess, Cell cell) {
        checkObjectIsNotNull(objectToProcess);

        if (cell == null) {
            throw new IllegalArgumentException("The cell cannot be null");
        }

        MetaData metaData = processors.get(cell.getColumnIndex());

        try {
            Method method = objectToProcess.getClass().getMethod(metaData.getMethodToExecute(),
                    metaData.getFieldType());

            method.invoke(objectToProcess, metaData.getProcessorToUse().process(cell));
        } catch (Throwable t) {
            throw new IllegalStateException(t);
        }
    }

    public void process(T objectToProcess, Cell... cells) {
        checkObjectIsNotNull(objectToProcess);

        if (cells == null || cells.length == 0) {
            throw new IllegalArgumentException("The cells cannot be empty");
        }

        for (Cell cell : cells) {
            process(objectToProcess, cell);
        }
    }

    public void process(T objectToProcess, Iterator<Cell> cellIterator) {
        checkObjectIsNotNull(objectToProcess);

        if (cellIterator == null) {
            throw new IllegalArgumentException("The iterator cannot be null");
        }

        while (cellIterator.hasNext()) {
            process(objectToProcess, cellIterator.next());
        }
    }

    public static <T> Processor<T> newInstance(Class<T> processorClass) {
        if (processorClass == null) {
            throw new IllegalArgumentException("The class cannot be null");
        }

        Map<Long, MetaData> processors = new ConcurrentHashMap<>();

        for (Field field : processorClass.getFields()) {
            if (field.isAnnotationPresent(ProcessWith.class)) {
                ProcessWith processWith = field.getAnnotation(ProcessWith.class);
                Long columnIndex = processWith.columnIndex();
                Class<? extends ExcelColumnProcessor<?>> clazz = processWith.processorClass();
                Class<?> fieldType = field.getType();

                try {
                    MetaData metaData = new MetaData();
                    metaData.setFieldType(fieldType);

                    String fieldName = field.getName();
                    metaData.setMethodToExecute(
                            "set" + Character.toUpperCase(fieldName.charAt(0)) + fieldName.substring(1));

                    metaData.setProcessorToUse(clazz.newInstance());

                    processors.put(columnIndex, metaData);
                } catch (Throwable t) {
                    throw new IllegalStateException(t);
                }
            }
        }

        Processor<T> processor = new Processor<>(processors);
        return processor;
    }

    private static class MetaData {

        private String methodToExecute;
        private Class<?> fieldType;
        private ExcelColumnProcessor<?> processorToUse;

        public String getMethodToExecute() {
            return methodToExecute;
        }

        public void setMethodToExecute(String methodToExecute) {
            this.methodToExecute = methodToExecute;
        }

        public Class<?> getFieldType() {
            return fieldType;
        }

        public void setFieldType(Class<?> fieldType) {
            this.fieldType = fieldType;
        }

        public ExcelColumnProcessor<?> getProcessorToUse() {
            return processorToUse;
        }

        public void setProcessorToUse(ExcelColumnProcessor<?> processorToUse) {
            this.processorToUse = processorToUse;
        }

    }

}

The factory method looks for the fields marked with the annotation and saves the information to process that field in a MetaData object. Then, when the actual processing needs to be done I need the object where to inject the values from the cells of the excel and the cell itself.

I'm relying on the notion that the object in which to inject the values has mutators for the fields that need processing. Also, I'm always instantiating a processor class for each field, which it isn't the best right now because some of the processors might be reused for multiple columns.

I'm concerned about performance. I've read that using reflection usually is pretty slow, so I was wondering if someone knows a better way to do this.

\$\endgroup\$
4
  • \$\begingroup\$ "A better way to do this" or do you mean "a better way to do this in java"? Believe it or not I did almost the same task in assembly and both are the wrong tools for the job; in java, static typing is making more work for you than you need. In assembly, you don't deal with static typing, but everything else is slightly more complicated. It actually worked out rather nicely. Anyway. You are basically building dynamic type in java from scratch. Python or JS understands the concept much better and will not be so much of an uphill battle. \$\endgroup\$
    – sqykly
    Commented Nov 6, 2015 at 1:57
  • \$\begingroup\$ Don't forget that excel will save to csv or tsv, and you can read those with anything. Unless you really need to work with the data in the context of a bunch of other java code... well, think of it this way: each row is an object, the columns are its keys, which makes the header row a class. You're implementing dynamic classes; how much of that do you want to rewrite in java when other languages already have dynamic objects that fit the purpose out of the box? \$\endgroup\$
    – sqykly
    Commented Nov 6, 2015 at 2:10
  • \$\begingroup\$ This excel processor component is part of a much larger app, so I'm tied to Java. Using a statically typed language for this probably isn't the best, but I don't have much leverage to make a decision in changing the language of the implementation \$\endgroup\$
    – shulito
    Commented Nov 6, 2015 at 13:27
  • \$\begingroup\$ Do you actually need three process() methods? That is, is there a use case other than "populate the object with the entire row's values"? \$\endgroup\$
    – Eric Stein
    Commented Nov 6, 2015 at 15:54

1 Answer 1

1
\$\begingroup\$

Here's something for you to chew on. I made some assumptions about requirements which may or may not be valid.

  1. Don't bother with methods. Just go directly to the fields.
  2. Don't track structs (MetaData). Make real objects that do real work.
  3. Don't use in-out parameters (T objectToProcess) because they can be confusing.
  4. Recursion can be expensive, but I don't think you're going to be able to get away from it. Don't worry about it unless it's a demonstrated bottleneck causing a real performance problem. Trust Donald Knuth.
  5. I don't see how ConcurrentHashMap is buying you anything.
  6. You don't need to expose the single-cell process() method - the array method covers that case.
  7. You should have an abstraction-level exception that clients can handle if they choose to. In the code below, that exception is checked for visual clarity, but it could easily be made unchecked.
  8. It might be nice if you had a Row type. Then you could simplify Processor further to only accept a Row, rather than having to handle assorted input types.
  9. It might also be nice if you had a marker interface for a "processable" class. Then you could modify the generics to ensure that T was processable, rather than allowing non-processable instances. Alternately, you could have newInstance()/forClass() throw an exception if no annotations are found.

Putting it 1-7 together, here's something untested to play around with:

import java.lang.reflect.Field;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Objects;

public final class Processor<T> {

    private final Map<Long, DataInjector> dataInjectors;
    private final Class<T> clazz;

    public static <T> Processor<T> forClass(final Class<T> processorClass) throws ProcessorException {

        Objects.requireNonNull(processorClass);

        final Map<Long, DataInjector> processors = new HashMap<>();

        for (final Field field : processorClass.getFields()) {
            if (!field.isAnnotationPresent(ProcessWith.class)) {
                continue;
            }
            field.setAccessible(true);

            final ProcessWith processWith = field.getAnnotation(ProcessWith.class);
            final Long columnIndex = processWith.columnIndex();
            final Class<? extends ExcelColumnProcessor<?>> clazz = processWith.processorClass();

            processors.put(columnIndex, new DataInjector(field, newInstanceOf(clazz)));
        }

        return new Processor<>(processorClass, processors);
    }

    private Processor(final Class<T> clazz, final Map<Long, DataInjector> dataInjectors) {
        this.dataInjectors = Collections.unmodifiableMap(dataInjectors);
        this.clazz = clazz;
    }

    public T process(final Cell... cells) throws ProcessorException {
        Objects.requireNonNull(cells);
        if (cells.length == 0) {
            throw new IllegalArgumentException("The cells cannot be empty");
        }

        final T object = newInstanceOf(this.clazz);
        for (final Cell cell : cells) {
            this.injectData(object, cell);
        }
        return object;
    }

    public T process(final Iterator<Cell> cellIterator) throws ProcessorException {
        Objects.requireNonNull(cellIterator);

        final T object = newInstanceOf(this.clazz);
        while (cellIterator.hasNext()) {
            this.injectData(object, cellIterator.next());
        }
        return object;
    }

    private static <T> T newInstanceOf(final Class<T> clazz) throws ProcessorException {
        try {
            return clazz.newInstance();
        } catch (final InstantiationException | IllegalAccessException e) {
            throw new ProcessorException("Unable to create an instance of " + clazz, e);
        }
    }

    private void injectData(final T object, final Cell cell) throws ProcessorException {
        this.dataInjectors.get(cell.getColumnIndex()).inject(object, cell);
    }

    private static class DataInjector {

        private final Field field;
        private final ExcelColumnProcessor<?> columnProcessor;

        public DataInjector(final Field field, final ExcelColumnProcessor<?> columnProcessor) {
            this.field = field;
            this.columnProcessor = columnProcessor;
        }

        public void inject(final Object object, final Cell cell) throws ProcessorException {
            final Object value = this.columnProcessor.process(cell);
            try {
                this.field.set(object, value);
            } catch (final IllegalArgumentException e) {
                throw new ProcessorException(
                        "Value " + value + " from cell " + cell + " is not a valid type for field " + this.field, e);
            } catch (final IllegalAccessException e) {
                throw new ProcessorException("Unable to write to field " + this.field, e);
            }
        }
    }
}

This is all assuming you're really set on the annotation route. It's unclear to me this is superior to using classes to handle direct mapping. Any such investigation would probably have to be a new question, though.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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