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 void 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?

Related posts

Recent Viewed