1
\$\begingroup\$

What is wrong with this pattern which emulates AsyncTask functionality?:

// on the UI thread
final CountDownLatch latch = new CountDownLatch(1);
onPreexecuteWrapper();
new Thread(new Runnable() {
        doInBackgroundWrapper();
        latch.countDown();
    }
}).start();

new Thread(new Runnable() {
    @Override
    public void run() {
        try {
                latch.await();
        } catch (InterruptedException e) {
            Logger.error(TAG, "Not good.");
        }

    context.runOnUiThread(new Runnable() {
        @Override
        public void run() {
            onPostExecuteWrapper();
        }
    });
}
}).start();

Here's what I think is wrong:

  • Two threads -> more resources (I didn't compare actually, just guessing).
  • No progress update functionality.
  • Appears to be stinky.
\$\endgroup\$
3
\$\begingroup\$

First thing that is wrong is that you are reinventing a wheel.

Second why not ditch the CountDownLatch and do context.runOnUiThread directly in the first runnable instead?

// on the UI thread

onPreexecuteWrapper();
new Thread(new Runnable() {
    @Override
    public void run() {
        doInBackgroundWrapper();
        context.runOnUiThread(new Runnable() {
            @Override
            public void run() {
                onPostExecuteWrapper();
            }
        });
    }
}).start();

Last thing is that it's better to use an Executor and ThreadPool than to spawn new threads.

\$\endgroup\$
2
\$\begingroup\$

Appears to be stinky?? it stinks...

first thing I percieved as odd was the indentation:

@Override
public void run() {
    try {
            latch.await();
    } catch (InterruptedException e) {
        Logger.error(TAG, "Not good.");
    }

context.runOnUiThread(new Runnable() {

This code gives off the wrong impression that it stops after catching the InterruptedException.

It should looks like this:

@Override
public void run() {
    try {
            latch.await();
    } catch (InterruptedException e) {
        Logger.error(TAG, "Not good.");
    }

    context.runOnUiThread(new Runnable() {

scratch that... First let's read up in the Oracle "Concurrency Essentials":

An interrupt is an indication to a thread that it should stop what it is doing and do something else.

An InterruptedException basically is a DROP WHATEVER YOU DO. Continuing execution after that is basically moot. This means the catch block should be more along the lines of ...

} catch (InterruptedException e) {
     logger.warning("Got interrupted while waiting on latch");
     Thread.currentThread().interrupt();
}

Then again this whole construction presented here seems to be an overcomplicated way of achieving things.

Instead of needlessly creating Threads (iow. workers), to do tasks, you should have somebody that gives work that you have to some workers they have.

There's already existing solutions for that mentioned in ratchet freak's answer

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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