3
\$\begingroup\$

I have this piece of code where the class ExecutionMessage has three variants. The point is that the classes are autogenerated by Apache CXF and they do not have a common class. But all the three flavors have the same API. Is there a way to refactor the same using generics or Java 8 semantics to avoid the repetition of code.

public void convertToList (ExecutionMessageV2 mesg, List<ErrorDetail> errors) {
    if (mesg == null) {
        return;
    }
    ErrorDetail err = new ErrorDetail(mesg.getCode(),mesg.getDescription(), mesg.getSourceExceptionName());
    if (mesg.getSeverity() != null)
        err.setSeverity(mesg.getSeverity().value());
    if (mesg.getType() != null)
        err.setType(mesg.getType().value());
    errors.add(err);
    convertToList(mesg.getDetail(), errors);
}

public void convertToList (ExecutionMessageV1_0 mesg, List<ErrorDetail> errors) {
    if (mesg == null) {
        return;
    }
    ErrorDetail err = new ErrorDetail(mesg.getCode(),mesg.getDescription(), mesg.getSourceExceptionName());
    if (mesg.getSeverity() != null)
        err.setSeverity(mesg.getSeverity().value());
    if (mesg.getType() != null)
        err.setType(mesg.getType().value());
    errors.add(err);
    convertToList(mesg.getDetail(), errors);
}

public void convertToList (ExecutionMessageV2_0 mesg, List<ErrorDetail> errors) {
    if (mesg == null) {
        return;
    }
    ErrorDetail err = new ErrorDetail(mesg.getCode(),mesg.getDescription(), mesg.getSourceExceptionName());
    if (mesg.getSeverity() != null)
        err.setSeverity(mesg.getSeverity().value());
    if (mesg.getType() != null)
        err.setType(mesg.getType().value());
    errors.add(err);
    convertToList(mesg.getDetail(), errors);
}
\$\endgroup\$
  • \$\begingroup\$ Is this code ever supposed to be change manually? If not: why bothering about the code duplication? Code duplication is evil in manually maintained code, but in generated code it simply does not matter... \$\endgroup\$ – Timothy Truckle Mar 27 '17 at 21:22
  • 1
    \$\begingroup\$ classes ExecutionMessageV1_0, ExecutionMessageV2_0 and ExecutionMessageV2 are autogenerated. They are generated from different versions. They are identical structurally. They have the same contract by technically do not share a common interface. The code that I posted is the code that I typed. \$\endgroup\$ – randominstanceOfLivingThing Mar 28 '17 at 4:00
  • \$\begingroup\$ Are you able to change your code to rely on a wrapper class then? \$\endgroup\$ – h.j.k. Mar 30 '17 at 12:07
1
\$\begingroup\$

Are you able to change your code to rely on a wrapper class then? - myself

Going on a limb here, but do you want to consider using a wrapper class to encapsulate the fact that you have three similar generated classes, and provide a common way of accessing/modifying them? This assumes:

  1. The methods on the generated classes have the same parameter and return types.
  2. For the case of getDetail() method, that returns the same type as the input object, i.e. recursion.

If both conditions satisfy, then a sample wrapper implementation can be as such (using poorly named class names and variables, please excuse that):

public class WrapperClass {

    private final C1 o1;
    private final C2 o2;

    private WrapperClass(C1 o1, C2 o2) {
        this.o1 = o1;
        this.o2 = o2;
    }

    public String getCode() {
        return findAny(mapC1(C1::getCode), mapC2(C2::getCode));
    }

    // other getter methods

    private <T> T mapC1(Function<C1, T> mapper) {
        return Optional.ofNullable(o1).map(mapper).orElse(null);
    }

    private <T> T mapC2(Function<C2, T> mapper) {
        return Optional.ofNullable(o2).map(mapper).orElse(null);
    }

    private static <T> T findAny(Object... items) {
        return (T) Arrays.stream(items).filter(Objects::nonNull).findAny().orElse(null);
    }

    public static WrapperClass withC1(C1 o1) {
        return new WrapperClass(o1, null);
    }

    public static WrapperClass withC2(C2 o2) {
        return new WrapperClass(null, o2);
    }
}

You will need a map...(Function<?, T>) method for each oh your message class that performs the conversion, and the findAny(Object...) method to get the desired non-null value, if applicable. Each getter method, e.g. getCode() as shown, is relatively trivial to implement. In your case, since you are only dealing with three different classes (which is already a lot), you can also consider a simpler if-ladder instead of streaming through the values.

A slight benefit of the wrapper class is that it lets you perform additional conversions too, for example to an ErrorDetails object:

public final WrapperClass {

    // ...

    public ErrorDetails toErrorDetails() {
        ErrorDetails result = new ErrorDetails(getCode(), getDescription(),
                                                getSourceExceptionName());
        Optional.ofNullable(getSeverity())
                .map(Severity::value)
                .ifPresent(result::setSeverity);
        Optional.ofNullable(getType())
                .map(Type::value)
                .ifPresent(result::setType);
        return result;
    }

    // ...

}

Your current implementations of (convertToList()) will then just become:

public void convertToList(C1 o1, List<ErrorDetails> errors) {
    convertToList(WrapperClass.withC1(o1), errors);
}

public void convertToList(C2 o2, List<ErrorDetails> errors) {
    convertToList(WrapperClass.withC2(o2), errors);
}

private void convertToList(WrapperClass wrapper, List<ErrorDetails> errors) {
    errors.add(wrapper.toErrorDetails());
    convertToList(wrapper.getDetail(), errors);
}

Implementing the wrapper class's getDetail() will be quite similar, other than the slight difference that it should also return its own type:

public WrapperClass getDetail() {
    return new WrapperClass(mapC1(C1::getDetail), mapC2(C2::getDetail));
}
\$\endgroup\$
0
\$\begingroup\$

I'm not familiar wich Apache CXF, but I will suppose that you cannot change the generated code in any way. If you could, then the whole refactoring would be a matter of writing a common interface and adding implements ... to each of the Message classes. Then, if the classes don't have a common interface, you cannot use them in an abstract way.

The only way I can think of right now, which would reduce the amount of boilerplate code is reflection:

public void convertToList (Object mesg, List<ErrorDetail> errors) throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
    if (mesg == null) {
        return;
    }
    Method getCode = mesg.getClass.getMethod("getCode");
    Method getDescription = mesg.getClass.getMethod("getDescription");
    Method getSourceExceptionName = mesg.getClass.getMethod("getSourceExceptionName");
    String code = (String) getCode.invoke(mesg);
    String code = (String) getDescription.invoke(mesg);
    String code = (String) getSourceExceptionName.invoke(mesg);
    ErrorDetail err = new ErrorDetail(code, description, sourceExceptionName);
    // and so on...
}

The above is not tested, but you get the idea (also see the example below). I am guessing the types returned by the invoked methods so edit accordingly.

The above is very risky, as it relies heavily on some other system providing the message objects. Use it with caution. You can only hope that the objects will have the same interface - if not, the above method will throw all sorts of exceptions.


Example

class Message1 {
    public String getMessage() {
        return "Message 1";
    }
}

class Message2 {
    public String getMessage() {
        return "Message 2";
    }
}

public class Example {
    public static void main(String[] args) {
        Message1 message1 = new Message1();
        Message2 message2 = new Message2();
        try {
            invokeMethodAndPrintResult(message1, "getMessage");
            invokeMethodAndPrintResult(message2, "getMessage");
        } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
            e.printStackTrace();
        }
    }

    public static void invokeMethodAndPrintResult(Object object, String methodName) throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
        Method method = object.getClass().getMethod(methodName);
        System.out.println("Invoking " + method.getName() + " on " + object.getClass().getName() + ", result=" + method.invoke(object));
    }
}

Output:

Invoking getMessage on com.example.Message1, result=Message 1
Invoking getMessage on com.example.Message2, result=Message 2
\$\endgroup\$
  • \$\begingroup\$ I did think of proxies and reflection. Reflection breaks the typing and forces to use a procedural approach. I was wordering if there is something on the lines of Generics or Functional interfaces. \$\endgroup\$ – randominstanceOfLivingThing Mar 28 '17 at 4:02

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.