Wait between tasks with SingleThreadExecutor

Question

I am trying to (simply) make a blocking thread queue, where when a task is submitted the method waits until its finished executing. The hard part though is the wait.

Here's my 12:30 AM code that I think is overkill:

public void sendMsg(final BotMessage msg) {
    try {
        Future task;
        synchronized(msgQueue) {
            task = msgQueue.submit(new Runnable() {
                public void run() {
                    sendRawLine("PRIVMSG " + msg.channel + " :" + msg.message);
                }
            });
            //Add a seperate wait so next runnable doesn't get executed yet but
            //above one unblocks
            msgQueue.submit(new Runnable() {
                public void run() {
                    try {
                        Thread.sleep(Controller.msgWait);
                    } catch (InterruptedException e) {
                        log.error("Wait to send message interupted", e);
                    }
                }
            });
        }
        //Block until done
        task.get();
    } catch (ExecutionException e) {
        log.error("Couldn't schedule send message to be executed", e);
    } catch (InterruptedException e) {
        log.error("Wait to send message interupted", e);
    }
}

As you can see, there's alot of extra code there just to make it wait 1.7 seconds between tasks. Is there an easier and cleaner solution out there or is this it?

Answer 1

Ok heres a thought. You can use a ScheduledExceutorService, which will remember the last time you executed a runnable and will delay the next execution accordingly, upwards of the maximum sleep time (hard coded here at 1700).

    //@GuardedBy("msgQueue")
Date mostRecentUpdate = new Date();

public void sendMsg(final BotMessage msg) {
    try {
        Future task;
        synchronized (msgQueue) {               
            long delta = new Date().getTime() - mostRecentUpdate.getTime();
            task = msgQueue.schedule(new Runnable() {
                public void run() {
                    sendRawLine("PRIVMSG " + msg.channel + " :" + msg.message);
                }
            }, delta <= 1700 ?1700 : 0, TimeUnit.MILLISECONDS);

            mostRecentUpdate = new Date();
        }
        // Block until done
        task.get();
    } catch (ExecutionException e) {
        log.error("Couldn't schedule send message to be executed", e);
    } catch (InterruptedException e) {
        log.error("Wait to send message interupted", e);
    }
}

Answer 2

If your declaration is as follows :

ExecutorService msgQueue = Executors.newSingleThreadExecutor();

you can simply use this code to achieve what you are looking for :

msgQueue.submit(new Runnable() {
        public void run() {
            sendRawLine("PRIVMSG " + msg.channel + " :" + msg.message);
            try {
                Thread.sleep(Controller.msgWait);
            } catch (InterruptedException e) {
                log.error("Wait to send message interupted", e);
            }
        }
    })

as a single threaded executor can only execute one task at once.

Things to note :

1. There is no need to synchronize around submits to an ExecutorService
2. There is no need for a Future if you do not need the result of the execution
3. Your handling of the InterruptedException is a tad ropey.