Generic callback object, but I need the type parameter inside methods

Question

Inside my android app, I currently have methods that look like below code.
Since they all need a callback object that basically does the same thing and I would like to try to eliminate the duplicated code seen below.

The object that gets posted via the mBus.post(response); statement needs to be the specific type.

@Subscribe
public void onLeave(LeaveRequest event) {
 mRailsApi.leave(event.getEmail(), event.getPassword(),
 new Callback<LeaveResponse>() {
 @Override
 public void failure(RetrofitError arg0) {
 LeaveResponse response = new LeaveResponse();
 response.setSuccessful(false);
 mBus.post(response);
 }
 @Override
 public void success(LeaveResponse leaveResponse,
 Response arg1) {
 // need to create one since the api just returns a
 // header with no body , hence the response is null
 LeaveResponse response = new LeaveResponse();
 response.setSuccessful(true);
 mBus.post(response);
 }
 });
}
@Subscribe
public void onGetUsers(GetUsersRequest event) {
 mRailsApi.getUsers(mApp.getToken(), new Callback<GetUsersResponse>() {
 @Override
 public void failure(RetrofitError arg0) {
 GetUsersResponse response = (GetUsersResponse) arg0.getBody();
 response.setSuccessful(false);
 mBus.post(response);
 }
 @Override
 public void success(GetUsersResponse getUsersResponse, Response arg1) {
 getUsersResponse.setSuccessful(true);
 mBus.post(getUsersResponse);
 }
 });
}

Here is what I have come up with so far. It seems to work but I am wondering if there is a better solution.
One thing that bothers me is, that I have both the type parameter for the class and I am passing in the class to the constructor. It seems like I should not need to pass in the same info in two different ways.

The method calls become this:

 @Subscribe
 public void onLeave(LeaveRequest event) {
 System.out.println("inside api repo - making leave request");
 LeaveResponse response = new LeaveResponse();
 mRailsApi.leave(event.getEmail(), event.getPassword(),
 new RailsApiCallback<LeaveResponse>(mBus, LeaveResponse.class ));
 } 

And this is the callback class :

public class RailsApiCallback<T extends BaseResponse> implements Callback<T> {
 private Bus mBus;
 private Class mType;
 public RailsApiCallback(Bus bus, Class type) {
 super();
 mBus = bus;
 mType = type; 
 }
 @Override
 public void failure(RetrofitError retrofitError) {
 T response = (T) retrofitError.getBody();
 response.setSuccessful(false);
 mBus.post(mType.cast(response));
 }
 @Override
 public void success(T convertedResponse, Response rawResponse) {
 T response = null;
 try {
 response = (T) (convertedResponse != null ? convertedResponse : mType.newInstance());
 } catch (InstantiationException e) {
 e.printStackTrace();
 } catch (IllegalAccessException e) {
 e.printStackTrace();
 }
 response.setSuccessful(true);
 mBus.post(mType.cast(response));
 }
}

• nPn: I see the point of the "What you can and cannot do", but it makes it difficult for these old questions. I guess what I really should have done, is posted my updated code a while back , before it was answered, but it is hard to remember to follow up on unanswered questions
• skiwi: In your public void success(LeaveResponse leaveResponse, Response arg1) { method you are creating a new LeaveResponse and posting it to the bus, while doing nothing with the former. Is that intended behaviour?

Answer 1

One thing that bothers me is, that I have both the type parameter for the class and I am passing in the class to the constructor. It seems like I should not need to pass in the same info in two different ways.

The reason for why you have to do that is because of Type Erasure. Simply put, the generic class is only known during compile-time.

---

• This call is unnecessary in your constructor: super();
• All your fields should be marked with final.
• Class mType; should be Class<T> mType;

• I believe the mType.cast is unnecessary here:

  mBus.post(mType.cast(response));
  

  simply mBus.post(response); is enough. The eventbus will automatically detect the class by calling obj.getClass() and invoke the appropriate listeners.

  In fact, your response variable is already declared as T response; so I don't see what good casting it will do.

• nPn: Thanks for the review, I have to go back and see what I ended up with for this ... it was a while ago. I figured out the type erasure part, and realized that passing in a class was not so good. I think I ended up passing in an instance of a class rather than Blah.class
• Simon Forsberg: @nPn I don't see what good passing in an instance of the class does. You might want to post a follow-up question, I hope to answer quicker if you do :)

Answer 2

Here is my updated code:

@Subscribe
public void onLeave(LeaveRequest event) {
 System.out.println("inside api repo - making leave request");
 mRailsApi.leave(event.getEmail(), event.getPassword(),
 new RailsApiCallback<LeaveResponse>(mBus, new LeaveResponse() ));
} 
public class RailsApiCallback<T extends BaseResponse> implements Callback<T> {
 private Bus mBus;
 private T mResponse;
 public RailsApiCallback(Bus bus, T response) {
 super();
 mBus = bus;
 mResponse = response; 
 }
 @Override
 public void failure(RetrofitError retrofitError) {
 System.out.println(retrofitError.toString());
 T response = (T) retrofitError.getBody() != null ? (T) retrofitError.getBody() : mResponse ;
 response.setRawResponse(retrofitError.getResponse());
 response.setSuccessful(false);
 mBus.post(response);
 }
 @Override
 public void success(T convertedResponse, Response rawResponse) {
 System.out.println(rawResponse.getBody());
 T response = convertedResponse != null ? convertedResponse : mResponse ;
 response.setSuccessful(true);
 response.setRawResponse(rawResponse);
 mBus.post(response);
 }
}

I think the main change I did here was to pass in an instance of the response class rather than create a new instance of the class inside success method. I don't think there was really any type checking in the original code.

• Simon Forsberg: Yes, creating the object outside the generic class seems like a better idea. No need to instantiate with reflection anymore.