1

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.

IKavanagh
  • 6,089
  • 11
  • 42
  • 47
Alekhya Vemavarapu
  • 1,145
  • 1
  • 10
  • 26
  • 1
    You shouldn't be modifying the state of the UI components from outside the context of the Event Dispatching Thread, calling `progressBar.setIndeterminate(true);` inside the `doInBackground` method is a bad idea – MadProgrammer Oct 27 '15 at 23:18
  • Okay, but that is when my progressBar is to be updated. How can I do it ? Thanks for the response – Alekhya Vemavarapu Oct 28 '15 at 04:42
  • Have a look at this [example](http://stackoverflow.com/questions/23125642/swingworker-in-another-swingworkers-done-method/23126410#23126410) – MadProgrammer Oct 28 '15 at 04:44
  • Hi, changed it, still not working. thanks.. – Alekhya Vemavarapu Oct 28 '15 at 04:45
  • Consider providing a [runnable example](https://stackoverflow.com/help/mcve) which demonstrates your problem. This is not a code dump, but an example of what you are doing which highlights the problem you are having. This will result in less confusion and better responses – MadProgrammer Oct 28 '15 at 04:45
  • In FileManager.java, i have a function "replaceSelected" which rewrites contents of the input file into another file (converts character encoding to UTF-8), when I comment that call to that function, my code is working as expected. If I uncomment, my UI is again becoming unresponsive. Any ideas ? – Alekhya Vemavarapu Oct 28 '15 at 04:49
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/93558/discussion-between-alekhya-vemavarapu-and-madprogrammer). – Alekhya Vemavarapu Oct 28 '15 at 05:04

2 Answers2

3

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

MadProgrammer
  • 343,457
  • 22
  • 230
  • 366
0

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.

  • Can you please help me understand where to add this part of the code. Also, Task extends SwingWorker class (just making sure you did not miss that) – Alekhya Vemavarapu Oct 27 '15 at 17:05
  • @AlekhyaVemavarapu, I edited my answer because I did miss that ;) – marcbrouwer Oct 27 '15 at 17:33
  • 1
    So you say FileManager is going into a new thread ? How do I go about it then ? Am absolutely clueless how to handle this issue. Any help is highly appreciated. – Alekhya Vemavarapu Oct 27 '15 at 17:35
  • 1
    Can you add the FileManager code? Perhaps you could try reading one file with a FileReader and see if that works. – marcbrouwer Oct 27 '15 at 17:38
  • 3
    Another thing to try is to pass the File to your Task and make and run the FileManager from the Task. The FileManager is made in the UI thread and only the call to read is done in the Task. That might be the problem. – marcbrouwer Oct 27 '15 at 17:42
  • FileManager is my class which does 1. Go through all of my input files & give a total count, 2. Prepare a list of all the files – Alekhya Vemavarapu Oct 27 '15 at 17:45
  • Tried sending only the file to Task, no luck. Code edited – Alekhya Vemavarapu Oct 27 '15 at 17:56
  • @BharadwajVemavarapu I cant comment or your answer, but that could be the problem. I think it could work by using a Handler – marcbrouwer Oct 27 '15 at 18:17
  • Thanks for your time.. Made some minor changes to the code & it started working. – Alekhya Vemavarapu Oct 27 '15 at 18:29
  • @alekhya I feel the main issue is the parent thread (UI) spawned a swingWorker. Progress bar is in UI(parent Thread). Can, swing worker update the progressbar? Refer to this article. http://www.javacreed.com/swing-worker-example/ – Bharadwaj Vemavarapu Oct 27 '15 at 18:05
  • 1
    The code isn't "fine", it's violating the single thread rules of Swing, modifying the UI from outside of the context of the EDT :P – MadProgrammer Oct 27 '15 at 23:20