patternjavaMinor
Simple, generic background thread worker class
Viewed 0 times
genericsimplebackgroundworkerthreadclass
Problem
While I'm not aiming for a drop-in replacement for
Considering the criticisms of
I believe I've used
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
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:
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:
Personally I hate that I have to put Java Thread creation and thread starting on two different lines!
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.