API Design – Designing a Flexible API with Support for Callbacks

api-designcallbacksdesigndesign-patternsjava

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?

Best Answer

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;
}
Related Topic