HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Simple, generic background thread worker class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
genericsimplebackgroundworkerthreadclass

Problem

While I'm not aiming for a drop-in replacement for AsyncTask, I would like a utility class that accomplishes some of the same goals.

Considering the criticisms of AsyncTask, in many cases I just deferred responsibility to the user - if the work you're pushing to a background Thread needs to be aware of your Activity lifecycle, save a reference to the AsynchronousOperation and explicitly cancel it in onPause, and make sure you're checking for - and reacting to - cancellation in the performWorkInBackgroundThread method. If you want to send results back to the main thread, use the provided runOnUiThread method.

I believe I've used volatile and synchronized blocks correctly here, but would be happy to hear any feedback, even if it's just to point out that I've done it badly.

Other than that - does this feel useful? Safe? Any obvious places it could be improved?

```
import android.os.Handler;
import android.os.Looper;
import android.os.Process;

import java.util.concurrent.BlockingDeque;
import java.util.concurrent.LinkedBlockingDeque;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;

/**
* Usage:
* new AsynchronousOperation(){
* @Override
* public void performWorkInBackgroundThread(){
* // do stuff...
* // if you can break things into small steps, do so -
* // e.g., maybe download by looping through chunks instead of a library method
* // at every opportunity, check #shouldQuit, if true, bail out
* // (or use the convenience #quitIfCancelledOrInterrupted)
* // for any operation you want to publish, use runOnUiThread and a Runnable
* // this would be for things like AsyncTask.onProgressUpdate or AsyncTask.onPostExecute
* // remember to keep references to cancel if the operation depends on the lifecycle of a View or Activity
* }
* }.start();
*/
public abstract class AsynchronousOperation implements Runnable {

protected static final int INITIAL_POOL_SIZ

Solution

1) best practice lazy instantiation

A better lazy field instantiation pattern is the following: https://en.wikipedia.org/wiki/Double-checked_locking

Example:

static volatile ThreadPoolExecutor helper;
public ThreadPoolExecutor getHelper() {
    ThreadPoolExecutor result = sThreadPoolExecutor;

    if (result == null) {
        return result;
    }

    synchronized(this) {
        result = helper;
        if (result == null) {
            sThreadPoolExecutor = result = new ThreadPoolExecutor(...);
        }
    }

    return result;
}


The same pattern should be used for getHandler.

Or save yourself the trouble and just always create the ThreadPoolExecutor and Handler.

2) mCancelled

Are you sure you need the whole workflow surrounding mCancelled? Relying on isInterrupted might be enough.

3) runOnUiThread

Why is runOnUiThread an instance method? It might as well be static.

4) one line creation & storing

You could let your start method return this; to do the following:

AsynchronousOperation x = new AsynchronousOperation(){}.start();  
x.cancel(true);


Personally I hate that I have to put Java Thread creation and thread starting on two different lines!

Code Snippets

static volatile ThreadPoolExecutor helper;
public ThreadPoolExecutor getHelper() {
    ThreadPoolExecutor result = sThreadPoolExecutor;

    if (result == null) {
        return result;
    }

    synchronized(this) {
        result = helper;
        if (result == null) {
            sThreadPoolExecutor = result = new ThreadPoolExecutor(...);
        }
    }

    return result;
}
AsynchronousOperation x = new AsynchronousOperation(){}.start();  
x.cancel(true);

Context

StackExchange Code Review Q#128631, answer score: 2

Revisions (0)

No revisions yet.