Java Swing Worker for Progress Bar - UI remains unresponsive for a long time

Question

I developed an application on Windows using Java, Swing (window builder).
On a button click, my application will go to another class (FileManager.java file) to count the total number of files in the input folder (meanwhile progressBar will be in indeterminate mode). Once the number of files are known, progressBar maximum value is set.

Then I call convertToXLS(fileMgr) to read through the content of each file (1 kb) and update the progressBar as each file is read.

Here is the code for it:

public class xmlToXL {
            public static void main(String[] args) {
        javax.swing.SwingUtilities.invokeLater(new Runnable() {
            public void run() {
                xmlToXL window = new xmlToXL();
                window.frame.setVisible(true);
            }
        });
        private void initialize() {
            ...... some UI code ........
        btnConvertXmlTo.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {    
                try {
                    preConvertToXLS();
                    Task task = new Task(folderPath.getText());
                    task.execute();
                } catch (Exception e1) {
                    e1.printStackTrace();
                }
            }// end of actionPerformed method
        }); // end of action listened

}//end of initialize

    public void preConvertToXLS() {    //method to set few UI properties
        btnConvertXmlTo.setEnabled(false);
        progressBar.setVisible(true);
        progressBar.setStringPainted(true);
        progressBar.setIndeterminate(true);
        progressBar.setString("Calculating Total number of files...");
        progressBar.setForeground(new Color(0, 102, 0));
    }

    ParserUtils parUtils = new ParserUtils(); //class to parse XML files (in another .java file)

    private void convertToXLS(FileManager fileMgr) {
        try {
            int i=1;
            parUtils.reset();
            progressBar.setValue(0);
            List<File> files = fileMgr.getFiles();
            for(File file : files) {
                progressBar.setString("Reading " + i+ " of " + fileMgr.getSize()+ " files");
                parUtils.parseFileUsingDOM(file); // This will read content of the input file 
                progressBar.setValue(i++);
            }
            btnConvertXmlTo.setEnabled(true);


        } catch (Exception e) {

        } 
    }

    class Task extends SwingWorker<Void, Void> {
        private FileManager fileMgr;

        public Task(String srcPath) {
            this.fileMgr = new FileManager(new File(srcPath));

        }

        /*
         * Main task. Executed in background thread.
         */
        @Override
        public Void doInBackground() {
            try {
                progressBar.setIndeterminate(true);
                fileMgr.readFiles();
                progressBar.setIndeterminate(false);
                progressBar.setMaximum(fileMgr.getSize());
                convertToXLS(fileMgr);
            } catch (Exception e) {
                e.printStackTrace();
            }
            return null;
        }

        /*
         * Executed in event dispatching thread
         */
        @Override
        public void done() {
            Toolkit.getDefaultToolkit().beep();
            try {
            progressBar.setString("FileRead Successful");
            } catch (Exception e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }
        }
    }//end of task class
}//end of My class

My UI becomes unresponsive after fileMgr.readFiles();. It takes a minute or two, sometimes three and then executes convertToXLS(fileMgr).

FileManager.java

import XMLParsing.DetermineEncoding;

public class FileManager {

    public HashMap<String, ArrayList<String>> dirFiles = null;
    public ArrayList<String> dirNames = null;
    public int numberOfFiles;
    private File src;
    private List<File> files;

    public FileManager(File src) {
        this.src = src;
        dirNames = new ArrayList<String>();
        dirFiles = new HashMap<String, ArrayList<String>>();
        numberOfFiles = 0;
        files = new ArrayList<File>();
    }



    public int getSize() {
        return numberOfFiles;
    }

    public ArrayList<String> getDirectories(){
        return dirNames;
    }

    public List<File> getFiles() {
        Iterator it = dirFiles.entrySet().iterator();
        while (it.hasNext()) {
            Map.Entry pair = (Map.Entry) it.next();
            String folderName = (pair.getKey()).toString();
            ArrayList<String> FileNames = (ArrayList<String>) pair.getValue();
            if (FileNames != null) {
                for (String fileName : FileNames) {
                    if(replaceSelected(fileName)) {
                        File fXmlFile = new File(fileName);
                        files.add(fXmlFile);
                    }
                    else {
                    }
                }
            }
        }
        return files;
    }

    public void readFiles() throws IOException {
        readFiles(src);
    }

    private void readFiles(File folder) throws IOException {
        if (folder.isDirectory()) {
            ArrayList<String> fileNames = new ArrayList<String>();
            for (final File file : folder.listFiles()) {
                if (file.isDirectory()) {
                    readFiles(file);
                } else {
                    String fileName = (file.getPath()).toString();
                    if(fileName.toLowerCase().endsWith(".xml")) {
                        fileNames.add(file.getPath());
                        numberOfFiles = numberOfFiles + 1;
                        System.out.println(".");
                        if(!dirNames.contains(file.getParentFile().getName()))
                                dirNames.add(file.getParentFile().getName());
                    }
                }
            }
            dirFiles.put(folder.getName(), fileNames);
        }
    }

    private boolean replaceSelected(String filePath) {
        String line;
        String input = "";
        try {
            DetermineEncoding DE = new DetermineEncoding();
            String encoding = DE.getFileEncoding(filePath);
            InputStreamReader file = new InputStreamReader(new FileInputStream(
                    filePath), encoding);
            BufferedReader br = new BufferedReader(file);
            while ((line = br.readLine()) != null) {
                input += line.toString() + " ";
            }
            file.close();
            Writer out = new BufferedWriter(new OutputStreamWriter(
                    new FileOutputStream(filePath), "UTF-8"));
            out.append(input.trim());
            out.flush();
            out.close();
        } catch (Exception e) {
            return false;
        }
        return true;
    }

}

DetermineEncoding.java

import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;

import org.mozilla.universalchardet.UniversalDetector;

public class DetermineEncoding {

    public DetermineEncoding() {
        // TODO Auto-generated constructor stub
    }

    public String getFileEncoding(String fileName) throws IOException {
        byte[] buf = new byte[4096];
        java.io.FileInputStream fis = new FileInputStream(fileName);
        UniversalDetector detector = new UniversalDetector(null);
        int nread;
        while ((nread = fis.read(buf)) > 0 && !detector.isDone()) {
          detector.handleData(buf, 0, nread);
        }
        detector.dataEnd();
        String encoding = detector.getDetectedCharset();
        if (encoding != null) {
          return encoding;
        } else {
          return "";
        }


    }

}

Please help me identify the issue.

Answer 1

The basic problem is one of perception. You "think" that the UI isn't responding, when in fact, it's just waiting.

When you call readFiles, it goes through ALL the files you have previously scanned for, reads them and then writes them out again, all while the progress bar is in "determinate" mode, so it doesn't display anything.

What you need is some way for the FileManager to provide updates about it's progress back to your worker, but the worker goes through a number of other methods, which also have to provide progress notifications.

This would seem to hint at the need for some kind of Observer Pattern, where by the worker can be informed by other parts of the program when something has changed.

We also need to do all of this in a way which allows use to update the UI safely

Let's start with the observer...

public interface ProgressListener {
    public void progressChanged(double progress);
    public void setStatus(String text);
}

Pretty simple, it will notify you when the progress of status changes, allowing who ever is listening to make updates as they see fit.

The basic progress value is between 0-1, meaning that the listener doesn't actually care how many values you have, it only cares about your progress, this eliminates the need to try and update the progress bar about it's maximum value and instead, simply focus on the need to update the progress bar between 0-100

Now we need to make room for it in the rest of the API

private void convertToXLS(FileManager fileMgr, ProgressListener listener) {
    try {
        int i = 1;
        listener.progressChanged(0d);
        List<File> files = fileMgr.getFiles(listener);
        for (File file : files) {
            listener.setStatus("Reading " + i + " of " + fileMgr.getSize() + " files");
            parUtils.parseFileUsingDOM(file); // This will read content of the input file 
            listener.progressChanged(i / (double) files.size());
        }
        btnConvertXmlTo.setEnabled(true);

    } catch (Exception e) {
        e.printStackTrace();
    }
}

And FileManager#getFiles....

public List<File> getFiles(ProgressListener listener) {
    Iterator it = dirFiles.entrySet().iterator();
    int count = dirFiles.size();
    for (Map.Entry<String, ArrayList<String>> entry : dirFiles.entrySet()){
        count += entry.getValue() == null ? 0 : entry.getValue().size();
    }
    int index = 0;
    listener.setStatus("Processing files...");
    while (it.hasNext()) {
        Map.Entry pair = (Map.Entry) it.next();
        String folderName = (pair.getKey()).toString();
        ArrayList<String> FileNames = (ArrayList<String>) pair.getValue();
        if (FileNames != null) {
            for (String fileName : FileNames) {
                if (replaceSelected(fileName)) {
                    File fXmlFile = new File(fileName);
                    files.add(fXmlFile);
                } else {
                }
                index++;
                listener.progressChanged(index / (double)count);
            }
        }
    }
    return files;
}

Next, we need to update the Task to take advantage of it's progress support, we also need to allow for the ability to change the status of the progress bar.

This we can do through the publish/process methods, to send messages from the background thread to the EDT. We can also "cheat" a little and use it to send messages to change the indeterminate state of the progress bar (fyi: You could also use the property change listener support to do this as well, which might be a cleaner approach)

class Task extends SwingWorker<Void, String> {

    protected   static final String INDETERMINATE_ON = "indeterminate.on";
    protected   static final String INDETERMINATE_OFF = "indeterminate.off";

    private FileManager fileMgr;

    public Task(String srcPath) {
        this.fileMgr = new FileManager(new File(srcPath));

    }

    @Override
    protected void process(List<String> chunks) {
        for (String text : chunks) {
            if (INDETERMINATE_OFF.equals(text)) {
                progressBar.setIndeterminate(false);
            } else if (INDETERMINATE_ON.equals(text)) {
                progressBar.setIndeterminate(true);
            } else {
                progressBar.setString(text);
            }
        }
    }

    /*
         * Main task. Executed in background thread.
     */
    @Override
    public Void doInBackground() {
        try {
            publish(INDETERMINATE_ON);
            fileMgr.readFiles();
            publish(INDETERMINATE_OFF);
            convertToXLS(fileMgr, new ProgressListener() {
                @Override
                public void progressChanged(double progress) {
                    setProgress((int) (progress * 100d));
                }

                @Override
                public void setStatus(String text) {
                    publish(text);
                }
            });
        } catch (Exception e) {
            e.printStackTrace();
        }
        return null;
    }

    /*
         * Executed in event dispatching thread
     */
    @Override
    public void done() {
        Toolkit.getDefaultToolkit().beep();
        try {
            progressBar.setString("FileRead Successful");
        } catch (Exception e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }
}//end of task class

And finally, we need to add a PropertyChangeListener to the Task when we create it, so we can get the progress updates and update the progress bar...

task.addPropertyChangeListener(new PropertyChangeListener() {
    @Override
    public void propertyChange(PropertyChangeEvent evt) {
        String name = evt.getPropertyName();
        switch (name) {
            case "progress":
                int value = (int) evt.getNewValue();
                progressBar.setValue(value);
                break;
        }
    }
});

Simple :P

Answer 2

The code seems fine. The only thing I cant seem to check is the FileManager. With a FileReader it runs in a separate thread allowing user operations at the same time. So I guess the FileManager must be causing the problem.