AsyncTask - Am I using it right?

Question

I'm writing an Android app that displays random JPG images from the web. If a button is pressed, the app sends a GET request to http://imgur.com/random, and from the server's response it saves an URL to a JPG image as a String (see code).

I got this to work using StrictMode.setThreadPolicy, but I can't do it using AsyncTask.

I've tried to put the extraction of the .JPG address in doInBackground, and the actual process of changing the image in onPostExecute, like this:

public class MainActivity extends Activity {

    // Page "http://imgur.com/random" redirects users to a random page;
    // After the redirect, the URL will be something like "http://imgur.com/gallery/XXXXXX";
    // Actual image is at "http://i.imgur.com/XXXXXX.jpg"
    private final String WHERE = "http://imgur.com/random";
    private ImageExtractor ie;

    private class ATask1 extends AsyncTask<String, Void, String> {

        @Override
        protected String doInBackground(String... params) {
            // Go to WHERE and get a valid URL to a random .jpg image
            ie = new ImageExtractor();
            ie.extractImageURL(WHERE);
            return null;
        }

        @Override
        protected void onPostExecute(String result) {
            // Change the image in ImageView based on the new URL
            ImageView i = (ImageView)findViewById(R.id.imageDisplay);

            try {
                Bitmap bitmap = BitmapFactory.decodeStream((InputStream)new URL(ie.getImageURL()).getContent());
                i.setImageBitmap(bitmap); 
            } catch (Exception e) {
            }
        }
    }

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
    }

    // Method called when the button is pressed
    public void getNewImage(View v) {
        new ATask1().execute("");
    }
    ....
}

---

public class ImageExtractor {

    private final String USER_AGENT = "Mozilla/5.0";
    private String imageURL;

    public void extractImageURL(String address) {
        String returnedURL;

        try {
            // Create URL object based on passed argument
            URL url = new URL(address);

            // Send GET request
            HttpURLConnection conn = (HttpURLConnection)(url.openConnection());
            conn.setRequestMethod("GET");
            conn.setRequestProperty("User-Agent", USER_AGENT);

            // Response code of the answer
            conn.getResponseCode();

            // Create and save image URL as a string
            returnedURL = conn.getURL().toString();
            imageURL = returnedURL.replace("http://imgur.com/gallery/", "http://i.imgur.com/") + ".jpg";
        }
        catch (IOException e) {

        }
    }

    // Return image URL as a string;
    // http://i.imgur.com/XXXXXX.jpg
    public String getImageURL() {
        return imageURL;
    }
}

Right now, if I press the button, the address of a random .JPG is stored correctly in imageURL, but nothing happens after that. Is it correct what I've done so far? Do I need to make a new AsyncTask/Thread for the BitmapFactory part, or something like that? How exactly should this be implemented?

Thanks for your help!

Answer 1

You've got the general concept but I'm curious about this bit:

Bitmap bitmap = BitmapFactory.decodeStream((InputStream)new URL(ie.getImageURL()).getContent());

You're doing this in onPostExecute which is on the UI thread, and this looks awfully like a network request. Newer versions of Android will just flat out refuse to do this and throw a NetworkOnMainThreadException. It seems the only thing you're doing asyncly is the request to get the path to the image.

Answer 2

It looks ok to me. Just a thing: in the doInBackground() method you're returning null - Keep in mind that this is the parameter that your onPostExecute() method will receive, So basically String result will be null. But in your case, as far as you don't use it in an incorrect way, it's ok.

Answering your question: It's hard to tell where the error is, but the way you're handling your AsyncTask seems right to me. You don't need to declare a new Thread or AsyncTask to do the extractImageURL(WHERE) job, after all, AsyncTask is already a Thread and will do that in background. I'd consider putting several Log.d() instances across all your AsyncTask code to see where the error is, but as far as structure goes, it doesn't seem to have any issue.