2
\$\begingroup\$

As part of my recent transition from c# to java i decided to write a small converter implementation to translate DTO objects to entities. I'm fully aware of already existing solutions, especially in spring boot, but decieded to create this class as part of my learning process to write projects in java.

General note: I've used lombok to not write get and set methods by hand

The main goal for me was to create a simple to use class to convert DTOs or input objects into entities by mapping the fields between them. Ideally the implementation would allow me to extend the base class to implement special converters while still using the basic reflection mapping.

Main implementation

public class Converter<F, T> {
    private final static String SET_METHOD_PREFIX = "set";
    private final static String GET_METHOD_PREFIX = "get";
    
    private final List<Field> fieldsOfF;
    private final List<Field> fieldsOfT;

    private final List<Method> methodsOfF;
    private final List<Method> methodsOfT;

    private final Class<T> classToType;
    private final Class<F> classFromType;

    public Converter(Class<F> fromClass, Class<T> toClass) {
        this.classToType = toClass;
        this.classFromType = fromClass;

        this.fieldsOfF = getAllFields(fromClass);
        this.fieldsOfT = getAllFields(toClass);

        this.methodsOfF = getAllMethods(fromClass);
        this.methodsOfT = getAllMethods(toClass);
    }

    public T convert(F object) throws ConverterException {
        T instance = getInstance(classToType);

        for (Field item : fieldsOfF) {
            setValue(instance, getValue(object, item, methodsOfF), item, fieldsOfT, methodsOfT);
        }

        return instance;
    }

    public F convertBack(T object) throws ConverterException {
        F instance = getInstance(classFromType);

        for (Field item : fieldsOfT) {
            setValue(instance, getValue(object, item, methodsOfT), item, fieldsOfF, methodsOfF);
        }

        return instance;
    }

    private Object getValue(Object object, Field field, List<Method> methods) throws ConverterException {
        Optional<Method> optionalMethod;
        Object value = null;
        if (field.canAccess(object)) {
            try {
                value = field.get(object);
            } catch (IllegalArgumentException | IllegalAccessException e) {
                e.printStackTrace();

                throw new ConverterException(e.getMessage(), e);
            }
        } else {
            optionalMethod = methods.stream().filter(
                    x -> x.getName().equals((GET_METHOD_PREFIX + capitalizeFirstChar(field.getName()))) && x.canAccess(object))
                    .findFirst();
            if (optionalMethod.isPresent()) {
                try {
                    value = optionalMethod.get().invoke(object);
                } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
                    e.printStackTrace();

                    throw new ConverterException(e.getMessage(), e);
                }
            }
        }
        return value;
    }

    private void setValue(Object instance, Object value, Field item, List<Field> fields, List<Method> methods)
            throws ConverterException {
        Optional<Field> optionalField = fields.stream().filter(
                x -> x.getName().equals(item.getName()) && x.getType().equals(item.getType()) && x.canAccess(instance))
                .findFirst();
        if (optionalField.isPresent()) {
            try {
                optionalField.get().set(instance, value);
            } catch (IllegalArgumentException | IllegalAccessException e) {
                e.printStackTrace();
            }
        } else {
            Optional<Method> optionalMethod = methods.stream().filter(
                    x -> x.getName().equals((SET_METHOD_PREFIX + capitalizeFirstChar(item.getName()))) && x.canAccess(instance))
                    .findFirst();
            if (value != null) {
                if (optionalMethod.isPresent() && optionalMethod.get().getParameterCount() == 1
                        && optionalMethod.get().getParameters()[0].getType().isAssignableFrom(value.getClass())) {
                    try {
                        optionalMethod.get().invoke(instance, value);
                    } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
                        e.printStackTrace();

                        throw new ConverterException(e.getMessage(), e);
                    }
                }
            }
        }
    }

    private <E> E getInstance(Class<E> instanceClass) throws ConverterException {
        E instance = null;
        try {
            instance = instanceClass.getDeclaredConstructor().newInstance();
        } catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException
                | NoSuchMethodException | SecurityException e) {
            e.printStackTrace();

            throw new ConverterException(e.getMessage(), e);
        }
        return instance;
    }

    private <E> List<Field> getAllFields(Class<E> clazz) {
        if (clazz == null) {
            return Collections.emptyList();
        }

        List<Field> result = new ArrayList<>(getAllFields(clazz.getSuperclass()));
        List<Field> filteredFields = Arrays.stream(clazz.getDeclaredFields()).collect(Collectors.toList());
        result.addAll(filteredFields);
        return result;
    }

    private <E> List<Method> getAllMethods(Class<E> clazz) {
        if (clazz == null) {
            return Collections.emptyList();
        }

        List<Method> result = new ArrayList<>(getAllMethods(clazz.getSuperclass()));
        List<Method> filteredFields = Arrays.stream(clazz.getDeclaredMethods()).collect(Collectors.toList());
        result.addAll(filteredFields);
        return result;
    }

    private String capitalizeFirstChar(String value) {
        return value.substring(0, 1).toUpperCase() + value.substring(1);
    }
}

The main methods are convert(T object) and convertBack(F object). I've tried to keep the usage as simple as possible without any additional methods or setup steps.

Specific implementation

public class SampleEntityConverter extends Converter<SampleDTO, SampleEntity> {

    private Converter<SampleNestedDTO, SampleNestedEntity> nestedConverter = new Converter<>(SampleNestedDTO.class,
            SampleNestedEntity.class);

    public SampleEntityConverter() {
        super(SampleDTO.class, SampleEntity.class);
    }

    @Override
    public SampleEntity convert(SampleDTO object) throws ConverterException {
        SampleEntity instance = super.convert(object);
        if (object.nested != null) {
            instance.setNested(nestedConverter.convert(object.nested));
        }

        return instance;
    }

    @Override
    public SampleDTO convertBack(SampleEntity object) throws ConverterException {
        SampleDTO instance = super.convertBack(object);
        if (object.getNested() != null) {
            instance.nested = nestedConverter.convertBack(object.getNested());
        }
        return instance;
    }
}

Sample DTO and entity classes

The SampleDTO is a very simple DTO object with a abstract class as base to check if my implementation will handle inherited classes correctly

@ToString
public class SampleDTO extends AuditedDTO {
    public String field1;
    public Integer field2;
    public Boolean field3;
    public Instant field4;
    
    public SampleNestedDTO nested;
    
    public List<String> list1 = new ArrayList<>();
    
    public Map<String, Instant> map1 = new HashMap<>();
    
    public SampleDTO() {
        this.created = Instant.now();
        this.createdBy = "Test";
    }
}
@ToString
public class SampleNestedDTO {
    public String nestedField1;
    public Integer nestedField2;
    public Boolean nestedField3;
    public Instant nestedField4;
}
public abstract class AuditedDTO {
    public Instant created;
    public String createdBy;
    public Instant lastUpdate;
    public String lastUpdatedBy;
}

The SampleEntity object uses only get and set methods instead of public member variables to match our implementation of hiberante entities.

@ToString
public class SampleEntity extends AuditedEntity {
    @Getter
    @Setter
    private String field1;
    
    @Getter
    @Setter
    private Integer field2;
    
    @Getter
    @Setter
    private Boolean field3;
    
    @Getter
    @Setter
    private Instant field4;
    
    @Getter
    @Setter
    private SampleNestedEntity nested;
    
    @Getter
    @Setter
    private List<String> list1 = new ArrayList<>();
    
    @Getter
    @Setter
    private Map<String, Instant> map1 = new HashMap<>();
}
@ToString
public class SampleNestedEntity {
    @Getter
    @Setter
    private String nestedField1;
    
    @Getter
    @Setter
    private Integer nestedField2;
    
    @Getter
    @Setter
    private Boolean nestedField3;
    
    @Getter
    @Setter
    private Instant nestedField4;
}
@ToString
public abstract class AuditedEntity {
    @Getter
    @Setter
    private Instant created;

    @Getter
    @Setter
    private String createdBy;

    @Getter
    @Setter
    private Instant lastUpdate;

    @Getter
    @Setter
    private String lastUpdatedBy;
}

Usage

Now with my implementation of the converter simply using it as

Converter<SampleDTO, SampleEntity> converter = new Converter<>(SampleDTO.class, SampleEntity.class);

SampleDTO sampleDto = new SampleDTO();
sampleDto.field1 = "Sample String";
sampleDto.field2 = 1;
sampleDto.field3 = true;
sampleDto.field4 = Instant.now();
sampleDto.list1.add("String 1");
sampleDto.list1.add("String 2");
sampleDto.list1.add("String 3");
sampleDto.map1.put("Now", Instant.now());
sampleDto.map1.put("Tomorrow", Instant.now().plus(1, ChronoUnit.DAYS));
        
sampleDto.nested = new SampleNestedDTO();
sampleDto.nested.nestedField1 = "Sample nested String";
sampleDto.nested.nestedField2 = 2;
sampleDto.nested.nestedField3 = false;
sampleDto.nested.nestedField4 = Instant.now();

SampleEntity sampleEntity = converter.convert(sampleDto);
// sampleEntity.getNested() will be null and not converted with the baisc converter

will convert the sampleDto but this will not convert nested objects as it can only handle simple datatypes and collections. The special implementation as SampleEntityConverter will handle nested entites.

SampleEntityConverter converter = new SampleEntityConverter();

SampleDTO sampleDto = new SampleDTO();
sampleDto.field1 = "Sample String";
sampleDto.field2 = 1;
sampleDto.field3 = true;
sampleDto.field4 = Instant.now();
sampleDto.list1.add("String 1");
sampleDto.list1.add("String 2");
sampleDto.list1.add("String 3");
sampleDto.map1.put("Now", Instant.now());
sampleDto.map1.put("Tomorrow", Instant.now().plus(1, ChronoUnit.DAYS));
        
sampleDto.nested = new SampleNestedDTO();
sampleDto.nested.nestedField1 = "Sample nested String";
sampleDto.nested.nestedField2 = 2;
sampleDto.nested.nestedField3 = false;
sampleDto.nested.nestedField4 = Instant.now();

SampleEntity sampleEntity = converter.convert(sampleDto);
// sampleEntity.getNested() will be converted using the SampleEntityConverter

I've added a simple exception class ConverterException as

public class ConverterException extends Exception {

    private static final long serialVersionUID = -2846211687408157933L;
    
    public ConverterException(String message, Throwable cause) {
        super(message, cause);
    }

}

because exceptions should only occur during the design and writing of classes and not at production use runtime. This will make the handling of the different exceptions a bit simpler and keep the resulting code cleaner as there is only one type of exception to catch.

SampleEntity sampleEntity = null;
try {
    sampleEntity = converter.convert(sampleDto);
} catch (ConverterException e) {
    // handle exception
    e.printStackTrace();
}

Questions

Now to my questions:

  1. Are there any general improvements to my code in regards to naming conventions, type usage or any general errors I've made when using my knowledge of c# on java.
  2. Regarding the constructor methods of the Converter<F, T>, could this be a potential cause for performance problems when reading larger classes or deep inhiterances?
  3. Am I using the Field and Method methods correctly to check if a field can be set or a method can be called?
  4. Are there any caveats to use super.convert() in the specific implentations?

Thanks

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Java naming conventions seem to vary, however for boolean there's often some combination of is, has, can, either in the field name, or the accessor methods (not necessarily both). So in intellij if I have a field isField3 and generate getter/setters they're called getField3, setField3 not getIsField3 as you might expect. Various views: stackoverflow.com/q/5322648/592182 \$\endgroup\$
    – forsvarir
    Commented May 18, 2021 at 14:31
  • \$\begingroup\$ Is the naming dependent on the IDE or the frameworks used to generate these methods? When using lombok I've only seen the getIsField3 variant so far. \$\endgroup\$ Commented May 19, 2021 at 13:11
  • 1
    \$\begingroup\$ Seems to vary by company/IDE. You might want to look at: comp.nus.edu.sg/~cs2103/AY1617S1/contents/… in section "10. Boolean variables should be prefixed". I've added an update to my answer which may allow you to bypass the get/set methods. \$\endgroup\$
    – forsvarir
    Commented May 20, 2021 at 0:11

1 Answer 1

1
\$\begingroup\$

Boolean

As I've said in my comment, naming conventions in Java can vary, however particularly with boolean fields there may not be a direct get/set mapping to the property method names.

Mirrors

Your converter only really works if your DTO and your Entity have the same field names. Whilst this is often going to be the case, it may not always be true. One of the reasons for having the separation between the Entity and the DTO is so that you're able to vary these names if appropriate to the area you're exposing them (eg database naming conventions). Most frameworks allow you to work around this by using attributes to specify mapped column names for example. If you wanted to support different names you may be able to do something similar using attributes.

ConverterException

Are you expecting conversion errors to be recoverable? If not, consider inheriting from RuntimeException instead of Exception. This means that the clients don't need to handle the exception. It seems to me you should be able to figure out if you're going to have issues at compile time, so I'd be tempted to make it a RuntimeException and add a simple unit test for each conversion (simply to prove that convert/convertBack execute).

That said if I look at the getValue function, you have this branch:

optionalMethod = methods.stream().filter(
        x -> x.getName().equals((GET_METHOD_PREFIX + capitalizeFirstChar(field.getName()))) && x.canAccess(object))
        .findFirst();
if (optionalMethod.isPresent()) {

And similarly in setValue

if (optionalMethod.isPresent() && optionalMethod.get().getParameterCount() == 1
        && optionalMethod.get().getParameters()[0].getType().isAssignableFrom(value.getClass())) {

But you don't do anything if the method isn't there. It might be OK to ignore get/sets for fields, however it might not. If I have a DTO and an Entity that are supposed to have the same fields, but one of the getters has a name mismatch, so the conversion doesn't work I would want to know. I wouldn't want to have to try to track down where a null field4 came from, only to find out that it's because a getter had the wrong name. If you want to support missing/skipping fields, consider using an attribute to mark it as optional, or having a 'strict' mode that could be passed into your converter so it throws if it's asked to convert between incompatible types.

Early return

Rather than declaring a variable, trying, then returning, consider just returning directly from within the try block:

private <E> E getInstance(Class<E> instanceClass) throws ConverterException {
    try {
        return  instanceClass.getDeclaredConstructor().newInstance();
    } catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException
            | NoSuchMethodException | SecurityException e) {
        e.printStackTrace();

        throw new ConverterException(e.getMessage(), e);
    }
}

This is a personal thing, but I'm not a huge fan of printing the stacktrace when throwing an exception. Either you handle the exception (which could be printing it), or you leave it to the caller to decide what to do (they may want it printed, or they may want to log instead). Printing and throwing just seems like you're picking both sides of the fence...

setting Null

In your setValue, in one of your branches you eventually check if the value that you're going to set is equal to null and don't do anything if it is. Since you're creating a new instance, any value that can be null in the destination will already be set to null, so could setValue be checking for null straight away and skipping the rest of it's processing if the value to set is null?

Your questions...

  1. There's nothing about your code that stands out as 'this was written by a C# developer'.

  2. I don't think hierarchies are likely to be deep enough to create a significant performance overhead unless you're declaring lots of these converters that you're not using. You could overcome this with various patterns so that only actually create a single version of a given converter that you'd then reuse for each conversion.

  3. These seem fine to me, although I've not done extensive reflection within Java.

  4. Not that I'm aware of.

You may not need methods at all...

From what I can remember in .Net, you can directly access private members in a class through reflection. Which made me curious, so I did a bit of investigating. It turns out that you can do it in Java as well.

Once you've got access to the field, you can simply tell the field that it's accessible... so:

field.setAccessible(true);
if (field.canAccess(object)) { // will always be true
\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the explanation. I've thought about adding a custom annotation for field names, but had no time doing it so far. The RuntimeException one is interesting, and as it is not really necessary to recover after a ConverterExeption was thrown I think I'll add this. The null check on value was actually just there to prevent a NullPointer on value.getClass(), but again your're right about doing nothing when the value is null. With the early return it really comes down to personal preference. \$\endgroup\$ Commented May 19, 2021 at 13:05
  • \$\begingroup\$ And, as the other comment got to long, with the early return it really comes down to personal preference. I like having a single return statement at the end of any method instead of multiple return statements and potential exit points buried in multiple if-else branches. But this will lead to more complex methods, as more checks have to be implemented. \$\endgroup\$ Commented May 19, 2021 at 13:10

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.