4

I am writing a Java library that needs to make http GET && POST requests. There will be two types of users of this library:

  1. Those with understanding of callbacks and know how to use them.
  2. Those that....don't.

So please consider the following two methods:

public void methodWithCallback(ServiceCallback callback) {
    try {
        // GET && POST code that cannot really be abstracted to a separate method
    } catch (IOException e) {
        callback.onError(e);
        callback = null;
    }
    callback.onSuccess(response);
    callback = null;
}

public Response methodWithoutCallback() throws IOException {

    try {
        // again, GET && POST code that cannot really be abstracted to a separate method
    } catch (IOException e) {
        logger.log(e);
        throw e;
    }
    return response;
}

Above code seems dirty to me, mainly because it has duplication of code to send the request. Also if I were writing a lot of such kind of methods, my code will very soon double up to hardly maintainable spaghetti.

I considered allowing the client to set a callback by providing a method like setCallback(Callback callback), and then have one general method that notifies the callback if it is NOT null. But this also seems dirty because I would be then writing a lot of if null checks!

What are your thoughts on it? What would be the desirable design pattern here that you would recommend?

4
  • 3
    Can your users cope with Futures or Promises? Commented Jun 3, 2016 at 1:03
  • I doubt so...I am providing this library to computer science students at uni. While some of them might can, I think most of them are still newbie java students. And I was just thinking of providing both levels of students with easily accessible methods. But since you are suggesting a different approach, I assume my approach isn't really a recommended practice?
    – Joel Min
    Commented Jun 3, 2016 at 1:12
  • Well, you may have to settle for converting the asynchrony into asynchronous methods, then. Commented Jun 3, 2016 at 3:06
  • You could implement the callback-free version in terms of the callback version, by having it pass in a callback that throws the exception/saves the response
    – Bwmat
    Commented Jun 3, 2016 at 15:23

1 Answer 1

2

In my opinion, you can use one method (for each service function) and differentiate callbackful and callbackless version by either allowing null callback to be passed (idea#1) or by using callback to be registered by setCallback like your second idea (idea#2).

Callback as argument (idea#1)

public Response method(ServiceCallback callback) throws IOException {
    try {
        // GET && POST code that cannot really be abstracted to a separate method
        /* In your code you handle the response outside the try block,
           it can raise NullPointerException because you set callback 
           to null, and onSuccess is also called even it is an error. */
        return handleSuccess(response, callback);
    } catch (IOException e) {
        /* Return statement is needed because the method signature requires it. */
        return handleError(e, callback);
        /* If you use this two line version below, you can make handleError 
           return void. */
        // handleError(e, callback);
        // return null;
    }
}

private Response handleSuccess(Response response, ServiceCallback callback) {
    if (callback != null) {
        callback.onSuccess(response);
    }
    /* I personally prefer to also return the result even the callback exists;
       otherwise just move the line below into if block and return null here. */
    return response;
}

private Response handleError(IOException e, ServiceCallback callback) throws IOException {
    if (callback != null) {
        callback.onError(e);
    } else {
        logger.log(e); /* because I assume you want to log it */
        throw e;
    }
    /* I assume there is no response if error occurred. */
    return null;
}

Callback as a field (idea#2)

public Response method() throws IOException {
    try {
        // GET && POST code that cannot really be abstracted to a separate method
        return handleSuccess(response);
    } catch (IOException e) {
        return handleError(e);
    }
}

private Response handleSuccess(Response response) {
    if (this.callback != null) {
        this.callback.onSuccess(response);
    }
    return response;
}

private Response handleError(IOException e) throws IOException {
    if (this.callback != null) {
        this.callback.onError(e);
    } else {
        logger.log(e);
        throw e;
    }
    return null;
}
2
  • Thanks buddy, indeed helpful approach that I didn't think of :)
    – Joel Min
    Commented Jun 3, 2016 at 4:02
  • @JoelMin I updated my answer by adding reason why handling success inside try block, please read it. Thank you
    – fikr4n
    Commented Jun 3, 2016 at 4:15

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