Problems with SwingWorker

Question

This code actually works, but I think it could be written differently. I have a custom JDialog that I am using to display an indeterminate progress bar. Right I set the title, visibility, and location within my while loop. While this works, this I would think that it should be displayed right under where I create my worker, but when I do that it gives me an illegal start of type error. Any ideas? Thanks.

SwingWorker worker = new SwingWorker<Void, Void>(){
           //I am thinking this should work, but it doesn't
//              Progress.setTitle("Loading Repository");
//              Progress.setLocationRelativeTo(null);
//              Progress.setVisible(true); 
            @Override
            protected Void doInBackground() throws Exception {
                    while (runLoad.getState() != Thread.State.TERMINATED && !isCancelled()) {
                        try {
                            Progress.setTitle("Loading Repository");
                            Progress.setLocationRelativeTo(null);
                            Progress.setVisible(true);  
                            synchronized (this) {
                                Thread.sleep(2000);
                            }
                        } catch (InterruptedException e) {
                            JOptionPane.showMessageDialog(null,
                                    "Process interrupted: " + e.getMessage(), "Error", JOptionPane.ERROR_MESSAGE);
                        }
                    }
                    return null;
                }
            @Override
            public void done() {
                SwingUtilities.invokeLater(new Runnable() {
                   @Override
                   public void run() {
                     Progress.setVisible(false);
                     Progress.dispose();
                    }
                 });
               }
            };
            worker.execute();

score 1 · Accepted Answer · answered Nov 27 '13 at 16:08

You can't put such statements inside a class directly. They must be inside a method. That's why the code doesn't compile if you uncomment these statements.

These statements shouldn't be inside the worker at all. They should be in the enclosing method, just before the worker is executed. Indeed, you want to initialize the progress dialog and make it visible only once, before the background process is started. And you can't do that in doInBackground(), because every access to Swing components must be done in the event dispatch thread, and not in the background thread.

The same goes for the call to JOptionPane.showMessageDialog(): it shouldn't be inside the doInBackground() method, since it must be done in the event dispatch thread. It shoud be inside the done() method, which should call get() to see if the background process was successful or not.

And the code in done() doesn't need to be enclosed into SwingUtilities.invokeLater(), since done() is executed in the event dispatch thread.

Finally, you should respect the Java naming conventions: variables start with a lower-case letter.

I did some modification to the code. What would the return type be of get() if doInbackground() returns null. I guess it would either be a String for an error or null if the thread completed? — yellowman, Nov 27 '13 at 20:20
Read the javadoc. `get()` will return the result of the background computation (i.e. null in this case) if it completes normally, and will throw an ExecutionException wrapping the exception of the background computation if the background computation threw an exception. — JB Nizet, Nov 27 '13 at 20:29

Problems with SwingWorker

1 Answers1