Does it make sense to split up an existing multi-method interface into several single method interfaces just to take advantage of lambdas?

Question

Say I have an existing callback interface that has multiple methods. To illustrate my point I use a callback the likes that you would see in code that performs some HTTP client operations:

public interface GetCallback<T> {
    public void onSuccess(T data);
    public void onAuthFailure();
    public void onError(RequestError error);

    //Potentially more methods like onPreGet etc...
}

And a method that makes a request would take an instance of this callback as an argument:

public void issueRequest(String url, GetCallback<T> callback) {
    // Implementation ...
}

As is obvious, this suffers with verbosity at the call site:

public void getData(){
    issueRequest("http://programmers.stackexchange.com", new GetCallback<String>(){
        public void onSuccess(String data) {/*Do Something*/}
        public void onAuthFailure(){/*Ask for credentials*/}
        public void onError(RequestError error){/*Show error message*/}

    });
}

I have something similar in my code and it has been working well. I use a DefaultGetCallback class that provides default implementation of the onAuthFailure and onError methods since the action I want to take in those cases is pretty much the same regardless of what resource I'm requesting.

The question is, does it make sense to refactor this into a class composed of a set of single-method interfaces in order to take advantage of the lambda syntax?

public class GetCallback<T>{
    public interface OnSuccessListener<T> {
        public void onSuccess(T data);
    }

    public interface OnAuthFailureListener {
        public void onAuthFailure();
    }

    public interface OnErrorListener {
        public void onError(RequestError error);
    }

    private OnSuccessListener mSuccess;
    private OnAuthFailureListener mAuthFailure;
    private OnErrorListener mError;

    public GetCallback<T>(OnSuccessListener<T> success) {
        this.mSuccess = success;
    }

    public GetCallback<T> withAuthFailure(OnAuthFailureListener authFailure){
        this.mAuthFailure = authFailure;
        return this;
    }

    public GetCallback<T> withError(OnErrorListener error){
        this.mError = error;
        return this;
    }

}

I might use a Builder pattern to construct the GetCallback here but that's besides the point. The call site now becomes:

public void getData(){
    issueRequest(
        "http://programmers.stackexchange.com", 
        new GetCallback<String>(s -> doSomething(s))
            .withAuthFailure(() -> askForCredentials())
            .withError(error -> showErrorMessage(error))
    );
}

One advantage is I can customize the behavior of individual "events" (success, error etc) in isolation, at the call site, rather than having to sub-class or to create an anonymous inner class.

But then, how much is too much? Especially given the fact that my current design already served me well?

Answer 1

Instead of building a facade wouldn't it make sense to do something more like this?

public interface SuccessListener<T> {

    default void onSuccess(T data) {
        // Completely ignore it.
    }

}

public interface AuthFailureListener {

    default void onAuthFailure() {
        // Completely ignore it.
    }

}

public interface ErrorListener {

    default void onError() {
        // Completely ignore it.
    }

}

// Retain the old API
public interface GetCallback<T> extends SuccessListener<T>, AuthFailureListener, ErrorListener {
    // Now empty - we are a conglmerate.
}

public static <T> void issueRequest(String url, SuccessListener<T> successListener, AuthFailureListener failureListener, ErrorListener errorListener) {
    // Implementation ...
}

public static <T> void issueRequest(String url, GetCallback<T> callback) {
    // Ensures backwards compatability.
    issueRequest(url, callback, callback, callback);
}

public void getData() {
    issueRequest("http://programmers.stackexchange.com", new GetCallback<String>() {

        @Override
        public void onSuccess(String data) {
        }

        @Override
        public void onAuthFailure() {
        }

        //@Override
        //public void onError() {
        // Can now leave out the ones you don't want.
        //}
    });
}

It is still compatible with your earlier API and internally who cares that the same object is a listener for all three events. It also opens up the possibilities of adding default implementations and allowing the chance to issue a request with only a SuccessListener.

Note: I tweaked the code to use default for even more niceness.

Answer 2

I would say, if there's a cost for your clients (the Users of your interface) to migrate to this another API, don't do it, it's not that ugly at all. If it's something new, or the cost of change is very low (few clients), go for it, as it would be much better for readability.

Another point is: like writing fluent interfaces, it's extremely verbose to write those functional interfaces for being able to use the lambdas, so if this an internal API I would go with the simplier straight forward implementation (without lambdas). In other hand, If this is a library that will be used for another systems, I would go with the lambda path. :)

Answer 3

You should not design interfaces based on the features of the implementation language but rather on the business model and on the OOP design principles.

A reason for splitting your interface would be a violation of Interface segregation principle (I from SOLID). This means it makes sense splitting if your GetCallback implementations usually don't need to implement all the methods.