Why would you use nested InvokeLater calls in Java?

Question

I'm refactoring some code that runs a multi-stage process. Each step is inside a nested java.awt.EventQueue.invokeLAter.... call. It looks a little like this:

   import java.awt.EventQueue;


public class NestedInvokeLater {

    /**
     * @param args
     */
    public static void main(String[] args) {
        java.awt.EventQueue.invokeLater(new Runnable() {

            @Override
            public void run() {
                changeTabPanel();
                copySomeFiles();
                enableNextButton1();
                upDateProgressBar(10);
                java.awt.EventQueue.invokeLater(new Runnable() {

                    @Override
                    public void run() {
                        readInFiles();
                        doSomethingToFiles();
                        upDateProgressBar(15);
                        java.awt.EventQueue.invokeLater(new Runnable() {

                            @Override
                            public void run() {
                                doSomethingElse();
                                upDateProgressBar(100);

                            }
                        });
                    }
                });

            }

        });

    };
}

I am new enough at Java that I don't get the point of nesting these calls to add 'jobs' to the EDT, and I'm not 100% confident with fiddling with these calls either. I think I understand what the invokeLater call does, and what each step does. Please correct me if this understanding is wrong:

invokeLater is used to add some invocation to the list of jobs to be done in the Event Dispatch thread. Java then deals with when/how each invocation is done, ensuring that the EDT and in turn the GUI doesn't lock as it performs jobs 'in the background'.

Nesting these calls says to me that we should queue a set of jobs, one of which is to queue something, which will queue some jobs....one of which is to queue something. But the first inner invocation is only ever queued once the previous job is done. Everything occurs sequentially (this is in line of my understanding of the whole process), but I don't see why you would use nested requests to queue jobs to do so. I would have, if I was writing this from scratch, have simply created functions for each invocation and called them in turn.

I recognise, being only a novice at Java I am probably missing something huge that makes this nesting important. But there is no documentation of this, and no commenting in the code about the nesting.

What am I missing? What, if anything is the point in this code?

Answer 1

There is no point in doing so many nested invocations. It is based on a good intention, but it's badly implemented.

If you want to do this properly, use a SwingWorker.

The documentation of SwingWorker has a neat example of how you should implement performing several tasks in the background of the application (the PrimeNumbersTask class showed there).

Edit: Here's an example of what you should do with SwingWorker in your case.

class SequentialInvoker extends SwingWorker<Void, Integer> {
    @Override
    public void doInBackground() {

        changeTabPanel();
        copySomeFiles();
        enableNextButton1();
        setProgress(10);

        readInFiles();
        doSomethingToFiles();
        setProgress(15);

        doSomethingElse();
        setProgress(100);
    }
}

To actually show the progress on a progress bar, take a look at the following code, copied from the SwingWorker documentation:

JTextArea textArea = new JTextArea();
JProgressBar progressBar = new JProgressBar(0, 100);
SequentialInvoker task = new SequentialInvoker();
task.addPropertyChangeListener(
    new PropertyChangeListener() {
        public  void propertyChange(PropertyChangeEvent evt) {
            if ("progress".equals(evt.getPropertyName())) {
                progressBar.setValue((Integer)evt.getNewValue());
            }
        }
    }); 

With this code, your progress bar will show the progress as the SwingWorker works.

Answer 2

There are three jobs that are used in invokeLater here. Each one does a costly thing, call updateProgressBar and then adds the next job to the EDT.

The thing is, if the code just continued to the next costly thing instead of calling invokeLater to do it, the EDT would not have the chance to repaint the progress bar to show the new value of it. This is probably why the work is broken in three invokelater calls.

Now, this is not what I would call a good code. This is pretty bad practice: one should not do a long process in the EDT because it blocks everything and makes the GUI unresponsive. This should be changed so that the process is done in a separate thread, and then only call invokeLater to update the progress bar.

Edit: To answer more generally the question in the title: there is almost never a sensible reason to nest calls to invokeLater. When you are doing this, you say "queue this job so that it is done in the same thread but later when you feel it would be good". So it gives a chance to the rest of the GUI to repaint itself, like here. But it only makes sense if you have a long running process in the EDT, which you should always avoid.

Answer 3

One advantage of doing it this way is that other queued up things get to run in between. So, in between the section that does changeTabPanel() and the part that does readInFiles(), the GUI will get to respond to the user clicking on a button etc...

The actual implementation is a bit of a confusing mess and illustrates (IMHO) why anonymous functions were not such a good idea. Your inclination to make the three parts "real" functions and call them sequentially is a good one. But, to maintain the same logic, what you really need to do is make them three runnables and have each invokeLater the subsequent one.

And @Cyrille is correct that doing these major tasks on the EDT is poor practice.

Answer 4

The code you posted makes absolutely no sense to me - you can just write everything sequentially because you have no parallel threads running which might post events on the EDT. You need the first invokeLater() though, as you use Swing components.

But as your code suggests you are doing some relatively lengthy operations: reading files, do something with them, ... You should run these methods in a new worker thread, NOT the EDT. And, in the run() method of these worker threads, you'll need a call to EventQueue.invokeLater() to have your GUI updated.