Is it justified to use Reflection in this use case to workaround design issue that i am not allowed to fix?

Question

Would the following use case be considered as justified for Reflection?

There are bunch of classes generated from XSDs (hundreds currently on project) which represent various Responses.

All of these Responses include common response data structure, rather then extending it.

When event such as timeout happens, i only need to set single String to specific value.

If these classes were extending common response structure i could always set this response code without reflection, but this is not the case.

Therefore i wrote simple utility for my services which uses reflection to get setter method for the String field and invoke it with predefined value. Only known alternative to me would be to have class specific methods which would duplicate code to handle timeout, with the only difference of returned Response class.

protected T handleTimeout(Class<T> timeoutClass) {
    try {
        T timeout = timeoutClass.newInstance();
        Method setCode = timeoutClass.getDeclaredMethod(SET_RESPONSE_CODE, String.class);
        setCode.invoke(timeout, Response.TIMEOUT.getCode());
        return timeout;
    } catch (InstantiationException | IllegalAccessException  | SecurityException | NoSuchMethodException | IllegalArgumentException | InvocationTargetException e) {
        e.printStackTrace();
        throw new RuntimeException("Response classes must have field: \"code\" !");
    }

}

Relevant fact:

• this setter method should never change as it would require rework of hundreds of interfaces

Could somebody point out if there are some pitfalls i have missed or if there is alternate solution for reflection which would achieve the same result ?

Edit: I simply have no authority to get any changes done on XSDs, so any solution would have to be done locally. There should be no problems with serializing such objects, as they are shared between components.

Answer 1

Firstly there is a standard, normal everyday solution as suggested by @kutschkem, specifically: declare an interface that only contains this one setter method and implement that interface in every class which requires it. This uses standard polymorphism to do exactly what you need.

I understand this requires changing the definition of a lot of classes (but the change is trivial - just add 'implements MytimeoutThing' to every class) - even for 1000's of classes this seems a fairly easy fix for me.

I think that there are real problems with reflection:

1. You are creating a secret interface to all your classes that must be supported but there is no contract for this interface - when a new developer wants to add a new class he has to magically know about the name and signature for this method - if he gets it wrong the code fails at run-time as the compiler doesn't know about this contract. (So something as simple as misspelling the setters name isn;t picked up by the compiler)

2. It's ugly, hidden and not clearly part of any particular part of the software. A dev maintaining ANY of these classes will find this function (the setter) notice that it is never being called and just delete it - after all no code in the rest of the project refers to that setter so it obviously isn't needed.

3. A whole lot of static analysis tools won;t work - for example in most IDE's you can establish all the places that specific function is called from and all the places that a specific function calls - obviously this kind of functionality is not available if you use reflection. In a project with hundreds of near identical classes I would hate to loose this facility.

Answer 2

The actual problem you are facing is that you have a lot of classes that should share a common abstraction between them (inheriting the same class or implementing the same interface), but they don't. Trying to keep it that way and designing around would basically be taking care of the symptoms instead of the cause and will likely cause more problems in the future.

I suggest to solve the root cause instead by making all the generated classes have a common interface / superclass. You do not have to do this by hand - as they are all generated it should be possible to change them automatically without much struggle.

Answer 3

I'd try an alternate solution for generating your classes from the xml schema, in preference over reflection.

You can supply xjc with a custom binding like this:

<?xml version="1.0" encoding="UTF-8"?>
<bindings xmlns="http://java.sun.com/xml/ns/jaxb"
      xmlns:xsi="http://www.w3.org/2000/10/XMLSchema-instance"
      xmlns:xjc="http://java.sun.com/xml/ns/jaxb/xjc"
      xsi:schemaLocation="http://java.sun.com/xml/ns/jaxb http://java.sun.com/xml/ns/jaxb/bindingschema_2_0.xsd" version="2.1">
    <globalBindings>
        <xjc:superClass name="XmlSuperClass" />
    </globalBindings>
</bindings>

and implement you XmlSuperClass like this:

@XmlTransient                     // to prevent that the shadowed responseCode be marshalled
public class XmlSuperClass {
    private String responseCode;         // this will be shadowed
    public String getResponseCode() {    // this will be overridden
        return responseCode;
    }
    public void setResponseCode(String value) { //overridden too
        this.responseCode = value;
    }
}

Invoking xjc like this:

xjc -extension -b <yourbinding.xjb> -cp <XmlSuperClass> <xmlschemas.xsd...>

will generate bound classes like:

@XmlRootElement(name = "whatever")
public class Whatever extends XmlSuperClass {
    @XmlElement(required = true)
    protected String responseCode;    // shadowing
    public void setResponseCode(String...) //overriding
}

Answer 4

To become objective again:

There is no mention of the object instantiation. If you would have postulated a constructor with a String code parameter:

T timeout = timeoutClass.getConstructor(String.class)
    .newInstance(Response.TIMEOUT.getCode());

Would the save critics arise? To a lower extent, as parametrized constructors are even more indeterminate. Let's await the voting here.

Interface is better looking though.

interface CodeSetter {
   void setCode(String code);
}

protected <T extends CodeSetter> handleTimeout(Class<T> timeoutClass) {
    try {
        T timeout = timeoutClass.newInstance();
        timeout.setCode(Response.TIMEOUT.getCode());
        return timeout;

Answer 5

Okay, so let's suppose you've got this generated code:

public class Response1 {
   public void setResponseCode(int code) {...}
}

public class Response2 {
   public void setResponseCode(int code) {...}
}

What you need to do then is write an interface:

public interface ResponseCodeAware { //sorry for the poor name
   public void setResponseCode(int code);
} 

Then you need to write a script that goes through all the generated code files and simply adds implements ResponseCodeAware after every class definition. (That's assuming that there are no interfaces implemented already, in that case you have to play around a bit with the string processing.)

So your generated and post-processed classes will now look like this:

public class Response1 implements ResponseCodeAware {
   public void setResponseCode(int code) {...}
}

public class Response2 implements ResponseCodeAware {
   public void setResponseCode(int code) {...}
}

Note that nothing else changed, so code that doesn't know about your interface (including serialization) should work exactly the same.

And finally we can rewrite your method:

protected T handleTimeout(Class<T extends ResponseCodeAware> timeoutClass) {
    try {
        T timeout = timeoutClass.newInstance();
        timeout.setResponseCode( Response.TIMEOUT.getCode() );
        return timeout;
    } catch (InstantiationException | IllegalAccessException  | SecurityException | NoSuchMethodException | IllegalArgumentException | InvocationTargetException e) {
        e.printStackTrace();
        throw new RuntimeException("Response class couldn't be instantiated.");
    }
}

As you can see, unfortunately we still have to use reflection to create our object, and unless we also create some kind of factory, that will stay that way. But code generation can help you here too, you can build up a factory class in parallel to marking the classes with the interface.