How do I refactor my code to use AsyncTask?

Question

I made an application for Android that originally targeted a lower version (2.3). After I got my proof-of-concept working, I tried to get it to work on Android 4. That's when I got the NetworkOnMainThread exception.

After doing some research, I quickly found the AsyncTask, which sounded awesome. The problem is, I'm having a hard time wrapping my mind around it. For instance, here's my original code:

public void Refresh(Context c)
{
    SummaryModel model = MobileController.FetchSummary(c);

    TextView txtCurrentWeight = (TextView)findViewById(R.id.txtCurrentWeight);
    TextView txtWeightChange = (TextView)findViewById(R.id.txtWeightChange);
    TextView txtAvgPerWeek = (TextView)findViewById(R.id.txtAvgPerWeek);

    if(model.ErrorMessage == "")
    {
        txtCurrentWeight.setText(model.CurrentWeight);
        txtWeightChange.setText(model.WeightChange);
        txtAvgPerWeek.setText(model.Average);
    }
    else
    {
        Toast.makeText(c, model.ErrorMessage, Toast.LENGTH_LONG).show();

        txtCurrentWeight.setText("");
        txtWeightChange.setText("");
        txtAvgPerWeek.setText("");
    }
}

I created an AsychTask like this:

public class WebMethodTask extends AsyncTask<Object, Integer, Object> {
    @Override
    protected void onPreExecute() {
        super.onPreExecute();
    }

    @Override
    protected void onProgressUpdate(Integer... values) {
        super.onProgressUpdate(values);
    }

    @Override
    protected void onPostExecute(Object result) {
        super.onPostExecute(result);

        SummaryModel model = (SummaryModel)result;
        // Can't seem to access UI items here??

    }

    @Override
    protected Object doInBackground(Object... params) {

        Context c = (Context)params[0];
        return MobileController.FetchSummary(c);
    }
}

How do I access the UI items from the onPostExecute method? Or, do I have the wrong idea on how to use AsyncTask?

Thanks!

I think you got the idea OK. You can access UI from `onPreExecute` and `onPostExecute` only, but never from `doInBackground`, since that method runs in a different thread from the UI. On the other hand, take care and always check `if(!isCancelled())` from onPostExecute, just in case the Activity was terminated before the `doInBackground` method finishes and then prevent to access UI from a non-existing Activity. — thelawnmowerman, Jan 28 '13 at 15:20
Extra detail: findViewById is not defined within my AsyncTask. I'm not sure what the proper technique is for accessing it. Do I need to pass in the Activity object or what? — Doug Dawson, Jan 28 '13 at 15:21
Yes, you must pass a reference of the view to the constructor of WebMethodTask, or even better and easier, you can define WebMethodTask as an inner private class of the Activity that uses it and just define the reference of the view as global in the parent class that extends Activity. — thelawnmowerman, Jan 28 '13 at 15:33
And don't forget to cancel the thread from `onDestroy` method of the Activity (so you can check `if(!isCancelled())` later from the AsyncTask as I told before). — thelawnmowerman, Jan 28 '13 at 15:35

Orabîg · Accepted Answer · 2013-01-28T15:25:33.630

You should be able to accessUI where you put your comments (in the postExecute method)

Additionally, I would suggest to use more specialized class with for AsyncTask, so that your code looks better :

public class WebMethodTask extends AsyncTask<Object, Integer, SummaryModel> {

    private Activity source;
    public WebMethodTask(Activity activity) {
        this.source=activity;
        }

    @Override
    protected void onPreExecute() {
        super.onPreExecute();
    }

    @Override
    protected void onProgressUpdate(Integer... values) {
        super.onProgressUpdate(values);
    }

    @Override
    protected void onPostExecute(SummaryModel model) {
        super.onPostExecute(model );

        TextView txtCurrentWeight = (TextView)source.findViewById(R.id.txtCurrentWeight);
        TextView txtWeightChange = (TextView)source.findViewById(R.id.txtWeightChange);
        TextView txtAvgPerWeek = (TextView)source.findViewById(R.id.txtAvgPerWeek);

        if(model.ErrorMessage.length()==0)
        {
            txtCurrentWeight.setText(model.CurrentWeight);
            txtWeightChange.setText(model.WeightChange);
            txtAvgPerWeek.setText(model.Average);
        }
        else
        {
            Toast.makeText(c, model.ErrorMessage, Toast.LENGTH_LONG).show();

            txtCurrentWeight.setText("");
            txtWeightChange.setText("");
            txtAvgPerWeek.setText("");
        }

    }

    @Override
    protected SummaryModel doInBackground(Context ... params) {
        Context c = params[0];
        return MobileController.FetchSummary(c);
    }
}

Edit : Added a reference to your activity, to take your last comment into account.

However, if you acynctask can be long, it's maybe not a very good idea to keep a reference on an activity.

It would be a better design to create a listenerclass that will accept some displayModel(CummaryModel) method, and whose responsability is to cal the setText methods if the activity has not been paused / stopped in the meanwhile...

The constructor change is exactly what I was missing! Thanks! — Doug Dawson, Jan 28 '13 at 16:27

score 0 · Answer 2 · answered Jan 28 '13 at 15:13

0

Fill the ui items with the loaded model data in the WebMethodTask#onPostExecute method.

answered Jan 28 '13 at 15:13

endian

4,761
7
32
54

I would like to, but findViewById is not defined in my AsyncTask class. Do I need to pass a reference to the Activity? – Doug Dawson Jan 28 '13 at 15:19
yes, either you pass a activity instance or you pass the ui controls – endian Jan 28 '13 at 15:56

score 0 · Answer 3 · answered Jan 28 '13 at 15:35

0

You need a reference to your UI controls. When passing references to your UI controls to the ASyncTask you will create problems. Assume the following scenario:

show activity (activity instance 1)
call async task with te activity as reference.
rotate your device (by default a device rotation will create a new activity) -> (activity instance 2)
when the sync task is finished, activity instance 1 is used to display the results. However the activity no longer exists causing exceptions.

The conclusion is that the ASyncTask should not be used for network activity related background tasks.

Fortunately there is a solution: RoboSpice. RoboSpice uses another approach. Look at https://github.com/octo-online/robospice/wiki/Understand-the-basics-of-RoboSpice-in-30-seconds for a good explanation. More information: https://github.com/octo-online/robospice

answered Jan 28 '13 at 15:35

userM1433372

5,345
35
38

Sorry, but you are completely mistaken in your conclusion... You need to understand activities lifecycle better, because you can always control and prevent all those common situations with AsyncTasks. – thelawnmowerman Jan 28 '13 at 15:38
Please specify my misunderstanding? – userM1433372 Jan 28 '13 at 15:54
In the case of the code above, can I simply check that the Activity still exists before attempting to update Views on it or do I need something more robust? – Doug Dawson Jan 28 '13 at 16:30
On https://github.com/downloads/octo-online/robospice/RoboSpice-droid-con-amsterdam.pptx you can download a presentation with more details (including example) about the design problems with AsyncTasks and the solution RoboSpice provides. – userM1433372 Jan 28 '13 at 19:49
Configuration changes should always be taken in consideration in any Android app. Using AsyncTask without checking any of the problematic scenarios will cause problems, of course, but that's NOT a reason to conclude that AsyncTasks shouldn't be used for network background tasks. That's a misunderstanding. The only solution you provide is to use a third party library, and that's DEFINETELY another misunderstanding. Android provides many ways to deal with background tasks, and AsyncTasks are just one of them, and if you understand them, they are perfectly OK. – thelawnmowerman Jan 29 '13 at 00:30
That's correct. You can solve the problems yourself. Perhaps it had been better to write "there are far better solutions than the AsynTask for network communication" instead of "should not be used for network communication". Perhaps I had to mention the new Loaders (Googles attempt to repair the ASyncDesing flaw) introduced in Android 3 too. Some nice further reading: http://stackoverflow.com/questions/3357477/is-asynctask-really-conceptually-flawed-or-am-i-just-missing-something http://goo.gl/njX3a http://goo.gl/jEOQq – userM1433372 Jan 30 '13 at 07:22

score 0 · Answer 4 · answered Jan 28 '13 at 15:55

create an inner class in refresh method as

enter code herepublic void Refresh(Context c)

{ SummaryModel model = MobileController.FetchSummary(c);

TextView txtCurrentWeight = (TextView)findViewById(R.id.txtCurrentWeight);
TextView txtWeightChange = (TextView)findViewById(R.id.txtWeightChange);
TextView txtAvgPerWeek = (TextView)findViewById(R.id.txtAvgPerWeek);

if(model.ErrorMessage == "")
{
    txtCurrentWeight.setText(model.CurrentWeight);
    txtWeightChange.setText(model.WeightChange);
    txtAvgPerWeek.setText(model.Average);
}
else
{
    Toast.makeText(c, model.ErrorMessage, Toast.LENGTH_LONG).show();

    txtCurrentWeight.setText("");
    txtWeightChange.setText("");
    txtAvgPerWeek.setText("");
}
class WebMethodTask extends AsyncTask<Object, Integer, Object> {
@Override
protected void onPreExecute() {
    super.onPreExecute();
}

@Override
protected void onProgressUpdate(Integer... values) {
    super.onProgressUpdate(values);
}

@Override
protected void onPostExecute(Object result) {
    super.onPostExecute(result);

    SummaryModel model = (SummaryModel)result;
    // Can't seem to access UI items here??

}

@Override
protected Object doInBackground(Object... params) {

    Context c = (Context)params[0];
    return MobileController.FetchSummary(c);
}

}

How do I refactor my code to use AsyncTask?

4 Answers4