JProgressBar wont update

Question

I'm trying to add a JProgressBar to my program, but it wont update! The value only changes once it reasons 100%. Here's my method.

public void downloadImages(List<String> images) {
    if (errorCode == 0) {
        for (int i = 0; i < images.size(); i++) {
            if (errorCode == 0) {
                main.progressLabel.setText("Downloading image " + Integer.toString(i + 1) + " of " + Integer.toString(images.size()));
                String imageStr = images.get(i);
                String imageName = imageStr.substring(imageStr.lastIndexOf("/") + 1);
                try {
                    URL url = new URL(imageStr);
                    InputStream in = url.openStream();
                    OutputStream out = new FileOutputStream(saveDirectory + imageName);

                    byte[] b = new byte[2048];
                    int length;
                    while ((length = in.read(b)) != -1) {
                        out.write(b, 0, length);
                    }

                    in.close();
                    out.close();
                } catch (MalformedURLException e) {
                    errorCode = BAD_URL;
                } catch (IOException e) {
                    errorCode = INVALID_PATH;
                }
                main.progressBar.setValue(((i+1)/images.size())*100);
            }

        }
    }
}

Changing the progress bar value is at the bottom of the method above.

And here is how I call that method.

public void download() {
    final Downloader downloader = new Downloader(this, albumUrl.getText(), downloadPath.getText());
    progressBar.setValue(0);
    downloadButton.setEnabled(false);

    new Thread(new Runnable() {
        public void run() {
            List<String> images = downloader.getImages(downloader.getPageSource());
            downloader.downloadImages(images);
            if (downloader.getErrorCode() == 0) {
                progressLabel.setText("All images have been downloaded!");
            } else {
                String error = "";
                switch (downloader.getErrorCode()) {
                case Downloader.BAD_URL:
                case Downloader.NOT_IMGUR_ALBUM:
                    error = "The URL entered is either invalid or does not link to an Imgur album.";
                    break;
                case Downloader.BLANK_URL:
                    error = "The album URL field cannot be blank.";
                    break;
                case Downloader.INVALID_PATH:
                    error = "The system cannot find the download directory path specified.";
                    break;
                case Downloader.BLANK_PATH:
                    error = "The download directory cannot be blank.";
                    break;
                case Downloader.CANNOT_READ_URL:
                    error = "An error occoured while reading the URL.";
                    break;
                case Downloader.PARSING_ERROR:
                    error = "An error occoured while parsing the URL.";
                    break;
                }
                JOptionPane.showMessageDialog(Main.this, error, "Error", 0);
            }
            downloadButton.setEnabled(true);
        }
    }).start();
}

EDIT: The above was not the problem at all, the problem one the program was using Integer Division and not decimal.

Answer 1

Two rules of Swing...

1. Don't block the Event Dispatching Thread
2. Don't update any UI component from out side of the Event Dispatching Thread.

I'm sure there are more, but break either or both of these and expect lots-o-bad things to happen.

Swing is a single thread API. There is a single thread responsible for dispatching events, including repaint requests. If you block this thread (doing time consuming tasks, I/O, etc...) you will stop it from processing input from the user and dispatching repaint requests to the components - so nothing will update and your application will look like it's hung...

You need to get your image loading code OFF the EDT...

Take a look at Concurrency in Swing for more details, in particular, take a look at Worker Threads and SwingWorker

Answer 2

The main problem is, you are blocking the Event Dispatch Thread, by doing a long running task on the GUI EDT.

Rather use SwingWorker.

Here is a small example:

import java.awt.BorderLayout;
import java.awt.Cursor;
import java.awt.Insets;
import java.awt.Toolkit;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import java.util.Random;
import javax.swing.BorderFactory;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JProgressBar;
import javax.swing.JScrollPane;
import javax.swing.JTextArea;
import javax.swing.SwingWorker;

public class ProgressBarDemo extends JPanel {

    private JButton startButton;
    private JTextArea taskOutput;

    public ProgressBarDemo() {
        super(new BorderLayout());

        final JProgressBar progressBar = new JProgressBar(0, 100);
        progressBar.setValue(0);
        progressBar.setStringPainted(true);

        taskOutput = new JTextArea(5, 20);
        taskOutput.setMargin(new Insets(5, 5, 5, 5));
        taskOutput.setEditable(false);

        // Create the demo's UI.
        startButton = new JButton("Start");
        startButton.setActionCommand("start");
        startButton.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent ae) {
                startButton.setEnabled(false);
                setCursor(Cursor.getPredefinedCursor(Cursor.WAIT_CURSOR));
                // Instances of javax.swing.SwingWorker are not reusuable, so
                // we create new instances as needed.
                final Task task = new Task();
                task.addPropertyChangeListener(new PropertyChangeListener() {
                    @Override
                    public void propertyChange(PropertyChangeEvent pce) {
                        if ("progress".equals(pce.getPropertyName())) {
                            int progress = (Integer) pce.getNewValue();
                            progressBar.setValue(progress);
                            taskOutput.append(String.format("Completed %d%% of task.\n", task.getProgress()));
                        }
                    }
                });
                task.execute();
            }
        });

        JPanel panel = new JPanel();
        panel.add(startButton);
        panel.add(progressBar);

        add(panel, BorderLayout.PAGE_START);
        add(new JScrollPane(taskOutput), BorderLayout.CENTER);
        setBorder(BorderFactory.createEmptyBorder(20, 20, 20, 20));

    }

    /**
     * Create the GUI and show it. As with all GUI code, this must run on the
     * event-dispatching thread.
     */
    private static void createAndShowGUI() {
        JFrame frame = new JFrame("ProgressBarDemo");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        JPanel progressBarPanel = new ProgressBarDemo();
        frame.add(progressBarPanel);

        frame.pack();
        frame.setVisible(true);
    }

    public static void main(String[] args) {
        // Schedule a job for the event-dispatching thread:
        // creating and showing this application's GUI.
        javax.swing.SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                createAndShowGUI();
            }
        });
    }

    private class Task extends SwingWorker<Void, Void> {

        /*
         * Main task. Executed in background thread.
         */
        @Override
        public Void doInBackground() {
            Random random = new Random();
            int progress = 0;
            // Initialize progress property.
            setProgress(0);
            while (progress < 100) {
                // Sleep for up to one second.
                try {
                    Thread.sleep(random.nextInt(1000)-15);
                } catch (InterruptedException ignore) {
                }
                // Make random progress.
                progress += random.nextInt(10);
                setProgress(Math.min(progress, 100));
            }
            return null;
        }

        /*
         * Executed in event dispatching thread
         */
        @Override
        public void done() {
            Toolkit.getDefaultToolkit().beep();
            startButton.setEnabled(true);
            setCursor(null); // turn off the wait cursor
            taskOutput.append("Done!\n");
        }
    }
}

Answer 3

images.size() is an integer, and so is i+1, so what's happening is the decimal is getting truncated. What you should be doing so something like

main.progressBar.setValue((int)((i+1)/(double)images.size())/100))

What this'll do is it'll ensure that i+1 is being divided by a decimal capable data-type, which'll return the more inclusive data type (double in this case), and then it'll divide a double by an int which'll be no problem because it'll still return a double because it's more inclusive. We then cast that to an int because that's the data type setValue() wants.