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? Jun 3 '16 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
    Jun 3 '16 at 1:12
  • Well, you may have to settle for converting the asynchrony into asynchronous methods, then. Jun 3 '16 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
    Jun 3 '16 at 15:23
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
    Jun 3 '16 at 4:02
  • @JoelMin I updated my answer by adding reason why handling success inside try block, please read it. Thank you
    – fikr4n
    Jun 3 '16 at 4:15

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.