Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

In my Android app I have number of methods that should be executed in a background thread.

One of the methods looks like this

public static void processItem(final Context context, final String itemId)
{
    if (Looper.myLooper() == Looper.getMainLooper())
    {
        Thread task = new Thread()
        {
            @Override
            public void run()
            {
                processItem(context, playlistId);
            }
        };

        task.start();
        return;
    }

    // actual processing
}

As you can see, there are some code that only ensures an eligible thread is used.

How can I avoid copying this "threading" code over and over for each method that should be run in a background thread?

share|improve this question
    
You could use something like the GuiUtils class from this question : codereview.stackexchange.com/questions/52197/… (it's for javafx but the same principle apply) –  Marc-Andre Jul 8 at 13:09

3 Answers 3

up vote 6 down vote accepted

Extending Thread is an 'anti-pattern' in Java. The right way to do this work is to create an instance of a Runnable, and use the runnable as a constructor to a Thread instance.

Thread t = new Thread(new Runnable() {
    public void run() {
        // do something
    }
};

t.start();

But, and this is a big "but", in Java, in general, the practice of creating and using threads like this is being replaced by frameworks that manage the threads in a better way. In regular Java, you should be using the Executor, ExecutorService, and Executors class in the concurrent package.

As for android-specific use cases, you should read up on the threading models in Android paying special attention to the AsyncTask in Android dev kit

Update: more about the Thread anti-pattern

Jon Skeet says so ... ;-) and most people agree. What you have is something that needs to be done. That is what you need to implement, the way to do it. The place to run that something is on a thread. You do not implement the thread, you implement the task. There are two different concepts here: what needs to be done, and where it needs to be done. You are only implementing one of those.

Update: alternative solution

To reduce the logic required for managing the thread the code runs on, I would do one of two things:

  1. simply always use a different thread. Simply remove the if-condition and do all the processing in a Runnable's run method.
  2. Have a common utility method that looks something like:

    private static final ExecutorService THREADPOOL = Executors.cachedThreadPool();
    
    public static void runButNotOn(Runnable toRun, Thread notOn) {
        if (Thread.currentThread() == notOn) {
            THREADPOOL.submit(toRun);
        } else {
            toRun.run();
        }
    }
    

    and then you can call it in your method with:

    private static void processImplementation(final Context context, final String itemId) {
         // ... the actual work.
    }
    
    public static void processItem(final Context context, final String itemId) {
        Runnable task = new Runnable(){
            public void run() {
                processImplementation(context, itemId);
            }
        };
        runButNotOn(task, Looper.getMainLooper().getThread());
    }
    
share|improve this answer
    
AsyncTask is a good way to go here! ExecutorServices are available in Android as well and can also be used, as long as the UI-specific calls are wrapped in a post/runLater call. –  Simon André Forsberg Jul 8 at 15:22
    
Thank you for pointing to an anti-pattern in my code. It would be great to have a link to some explanation for why it is an anti-pattern. Btw, I don't see how your answer address duplication of the "threading" code. –  Bobrovsky Jul 9 at 10:40
    
@Bobrovsky - updated the answer with some detail –  rolfl Jul 9 at 11:45
    
Thank you for the great answer. It looks like using a shared ExecutorService and runnables is closest to what I like :-) –  Bobrovsky Jul 9 at 12:10

You can pass a Runnable to the processItem function which the only thing that does it to be sure that the Looper is the same as the main looper and if yes start a thread with the callback you passed to it.

processItem(Runnable callback);

will be:

public static void processItem(Runnable callback)
{
    if (Looper.myLooper() == Looper.getMainLooper())
    {
        new Thread(callback).start();
    }
}

And when you call it:

processItem(new Runnable() {
  @Override
  public void run() {
    What you want to do?
  }
});

P.S Without more context/info we can't help you really... And i think it's a question for Stackoverflow but i'm not sure.

share|improve this answer

Take a step back and look at your design decisions. You're talking about 'executing a method on a background thread'. What I'd like to propose, is to think about the task you're trying to achieve, and create the logic from there.

For example, your method processItem can perhaps belong to a class which handles threading for you:

/**
 * A class which processes item on a separate thread.
 */
public class ItemProcessor {

    private final Context mContext;
    private final String mItemId;

    public ItemProcessor(final Context context, final String itemId){
        mContext = context.getApplicationContext();
        mItemId = itemId;
    }

    /**
     * Starts processing the item on a separate thread.
     */
    public void process() {
        new Thread(new Runnable() {

            @Override
            public void run() {
                // Do the processing.
            }
        }).start();
    }

}

Think about modules in your application, don't just write linear code. For example, it is now a lot easier to add a callback interface to the class, using a setter method.

share|improve this answer
    
Thank you for the idea. –  Bobrovsky Jul 9 at 12:10

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.