jProgressBar update from SwingWorker

Question

I use to monitor a long running task by updating a ProgressBar. The long running task is of course performed in a Swingworker thread.

I used to program things like that :

public class MySwingWorkerClass extends SwingWorker<Void, Void> {   
    private JProgressBar progressBar;    

    public MySwingWorker(JProgressBar aProgressBar) {        
        this.progressBar = aProgressBar;           
        progressBar.setVisible(true);        
        progressBar.setStringPainted(true);
        progressBar.setValue(0);        
    }

    @Override
    public Void doInBackground() {
        //long running task
        loop {  
            calculation();
            progressBar.setValue(value);
        }
        return null;
    }    

    @Override
    public void done() {                
        progressBar.setValue(100);
        progressBar.setStringPainted(false);
        progressBar.setVisible(false);      
   }
}

but recently I discovered that I could do it by using the "setProgress" and defining the property change and do things like that

public class MySwingWorkerClass extends SwingWorker<Void, Void> {   
    private JProgressBar progressBar;    

    public MySwingWorker(JProgressBar aProgressBar) {        
        addPropertyChangeListener(new PropertyChangeListener() {
            public void propertyChange(PropertyChangeEvent evt) {
                if ("progress".equals(evt.getPropertyName())) {
                    progressBar.setValue((Integer) evt.getNewValue());
                }
            }
        });

        progressBar.setVisible(true);        
        progressBar.setStringPainted(true);
        progressBar.setValue(0);
        setProgress(0);
    }

    @Override
    public Void doInBackground() {
        //long running task
        loop {  
            calculation();
            setProgress(value);
        }
        return null;
    }    

    @Override
    public void done() {                
        setProgress(100);
        progressBar.setValue(100);
        progressBar.setStringPainted(false);
        progressBar.setVisible(false);      
   }
}

My question is : is my first code acceptable or shall I use the setProgress for anyreason ? I find the second code more complicated and in my case and don't know if there is any advantage or reason to use the second one.

Any advise ?

EDIT Thanks for the answer. As a summary. First solution is "wrong" because of the progress bar update is performed outside the EDT. Second solution is "correct" because the progress bar update is performed inside the EDT

Now, according to the "interesting" answer of @mKorbel in my case my calculation give results in HTML text which I "insert" (see this link). My current code is the following.

I publish(string) and my process code looks like that

@Override
    protected void process(List<String> strings) {
        for (String s : strings) {
            try {
                htmlDoc.insertBeforeEnd(htmlDoc.getElement(htmlDoc.getDefaultRootElement(), StyleConstants.NameAttribute, HTML.Tag.TABLE), s);
            } catch (BadLocationException ex) {
            } catch (IOException ex) {
            }
        }
    }

How can I reuse @mKobel to do the same in my case. I mean he use to override table rendering in my case what renderer shall I override (jTextPane?) and how ?

Answer 1

In the first code, you are calling the following line in a non-EDT (Event Dispatcher Thread) thread. So it is not thread safe:

progressBar.setValue(value);

This may result in unexpected behaviour as Swing is not designed as a thread-safe library.

There are different methods to perform this in the Swing way. One correct way of this is what you have done in the second post. Another would be to use publish()/process() methods, and a third method would be writing your own thread instead of SwingWorker and using SwingUtilities.invokeLater().