Generic asyncTask with callback (for web services)

Question

I've already developed many Android apps that make web service requests, always with the following approach:

In every activity that need to make a web service request, I define an inner AsyncTask that shows a ProgressDialog in onPreExecute(), makes the web service call in doInBackground, and dismisses the progressDialog and updates the results in the UI from onPostExecute().

My concern is: Is there a better (shorter) way to do it? Does it make sense to repeat all that code in every activity? I've been googling a lot, but I've found nothing.

My question is: Couldn't I define a Callback interface? for example this one:

public interface RequestCallback {
    public void onSuccess(Whatever whatever);
    public void onError(ErrorCode errorCode, String message);
}

... and then define an external class, for example AsyncRequest, that wraps the AsyncTask definition and the ProgressDialog show() and dismiss() statements. So, all activities would just need to instantiate that class, and pass in the following parameters:

1) The method of the web service to run

2) A Bundle with all the parameters of that method of the web service

3) A RequestCallback instance (that could be an anonymous inline instance, where I could update the UI from onSuccess())

4) The context of the Activity (necessary to show the ProgressDialog(), so I would still need a way to prevent configuration change exceptions and so...),

Do you find this a good design? It could save hundreds of lines of code...

Answer 1

Your approach is what I did on my project. And it saved a lot of code as you said, I don't have any complaint about it. But here is some issues that I want to tell you:

1. You should create new instance of AsyncTask every time you do a background thread to avoid to pile callback.

2. For the progress dialog, I use it as Singleton, because you don't show many dialogs at the same time. The dialog will be showed when you call the background job, and will be dismiss in the callback. Here is what I did:

   private void showProgressDialog(String strMess){
       if(null == progressDialog){
         progressDialog = new ProgressDialog(MainActivity.this);           
       }
       if(!progressDialog.isShowing()){
         progressDialog.setMessage(strMess);           
         progressDialog.show();
       }
   }
   
   private void hideProgressDialog(){
     if(null != progressDialog && progressDialog.isShowing()){
       progressDialog.dismiss();
     }
    }
   
   void someMethod(){
    showProgressDialog("Loading...");
    doBackgroundJob(param, new RequestCallBack() {
   
    public void onRequestCompleted(String message, boolean isSuccess) {
               hideProgressDialog();
               if(isSuccess){
   
               }else{
                   //do something on error
               }
           }
       });
   
     }
   

3. It is an optional, I defined an interface to notify instead of specific class, for each response I use one class, so in base class, I don't care what the response is. Here is it:

   public interface OnRequestCompleted<TResponse>  {
      void requestCompleted(TResponse response);
   }
   
   public abstract class BaseRequest<TResponse> implements IRequest{
         protected OnRequestCompleted<TResponse> delegate;
         protected Class<TResponse> responseClass;
   
         @Override
         public void send() {
            new HttpTask().execute();
         }
   
         private class HttpTask extends AsyncTask<Void, Void, String> {
         //...
   
         @Override
         protected void onPostExecute(String result) {
             if (null != response && null != delegate) {
             delegate.requestCompleted(response);
         }
        }
      }
   
     // the response example
     public class GroupResponse {
     public static class Clip {
             public int clipId;
         public String detail;
     }
   
     public static class Movie {       
     public int movieId;
             public String detail;
     }  
   
    }
   

   In the subclass of BaseRequest, I will tell it exactly what the response class is (Movie, Clip...)
   Hope this help.

Answer 2

If you use it already and it works for you, then yes it makes sense to make it generic and save the time (and bugs) of reimplementing the same thing dozens of times. If you ever find yourself copy-pasting large sections of code with few to no differences you should turn it into a library function or class of some sort. Otherwise if you find a problem later you'll have to fix it in a dozen places. It doesn't even matter if you think of a better way to do things later- its still easier to change it in one place than a dozen.

The only real issue I'd have with your solution is I wouldn't add the progress bar to it- I'd handle it in the calling code and the onSuccess/onError implementations. That way you could also reuse it for a background call that doesn't need to put up a UI. I try to keep my UI decisions as far away from data grabbing code as possible, MVC patterns are good.