Continuously execute runnables on the UI thread

Question

I need to run a sequence of runnables on the main (ui) thread with a given delay between them indefinitely until signalled to stop. Code:

public class UiLoop {

    private Handler h;
    private AtomicBoolean breakLoop;

    @UiThread
    public UiLoop() {

        if (Looper.myLooper() != Looper.getMainLooper()) {
            throw new RuntimeException("Not called on ui thread!");
        }

        h = new Handler();
        breakLoop = new AtomicBoolean(false);
    }

    /**
     * Break out of the loop on the next iteration.
     */
    public void breakLoop() {
        this.breakLoop.set(true);
    }


    /**
     * @param workers    A Deque of runnables to execute sequentially.
     * @param delay The delay between executing the runnables. This is excluding the time it
     *              takes for the runnable to complete.
     */
    @UiThread
    public void loop(Deque<Runnable> workers, int delay) {

        if (Looper.myLooper() != Looper.getMainLooper()) {
            throw new RuntimeException("Not called on ui thread!");
        }

        if (workers.isEmpty()) return;

        if (breakLoop.get()) return;

        final Runnable first = workers.removeFirst();
        h.post(first);
        workers.addLast(first);
        h.postDelayed(() -> loop(workers, delay), delay);

    }
}

Is the above code OK?

PS: I am using retrolambda to convert Anonymous Inner Classes to lambda functions.

Answer 1

1. Your variable names could be more clear. h is a poor choice, for example. loop is probably not a great name for a method either - maybe startLoop?

2. You have inconsistent code style. Some one-line if blocks are in curlies while others are not.

3. Why final Runnable first? It's fine if you want to use final wherever possible, but that doesn't appear to be the case with the rest of the code, and there's no explicit reason for it be final, so what is the strategy here?

4. I'd save the Runnable you pass to postDelayed as an instance member instead of creating a new one each time.

5. Why do you use an explicit this in this.breakLoop.set, but nowhere else? I'd choose one or the other.

6. If you've got those tests to ensure everything is happening on the main thread, then why h.post(first)? Wouldn't first.run() to the same thing?