10

I am using an MVC pattern for my design, when a user presses the search button, I call a search in the model, but I also want to update a progress bar with information returned from that model.

I have tried using a swingworker, but the progress bar does not update. I suspect I am doing something wrong with my threading.

My button as defined in the controller is:

 class SearchBtnListener implements ActionListener {
        public void actionPerformed(ActionEvent e) {
            _view.displayProgress();  
        }    
}

This calls the search in the model and has the following call in the view:

public void displayProgress() {

    TwoWorker task = new TwoWorker();
    task.addPropertyChangeListener(new PropertyChangeListener() {

         @Override
         public void propertyChange(PropertyChangeEvent e) {
             if ("progress".equals(e.getPropertyName())) {
                _progressBar.setValue((Integer) e.getNewValue());
             }
         }

     });
     task.execute();             
}      


private class TwoWorker extends SwingWorker<Void, Void> {        
    @Override
    protected Void doInBackground() throws Exception {
        _model.startSearch(getTerm());                  // time intensive code
        File file = new File("lock");           
        while (file.exists()){
            setProgress(_model.getStatus());
            System.out.println(_model.getStatus()); // never called
        }           
        return null;
    }  

    protected void done(){
        updateMain();
    }
}

Dummy function defined in Model for testing:

public int getStatus(){
    Random r = new Random();
    return r.nextInt();
}
Neutralise
  • 507
  • 8
  • 17

1 Answers1

12

Don't call

_progressBar.setValue(_model.getStatus());

from within your SwingWorker as this is calling Swing code from a background thread and is what the PropertyChangeListener is for anyway. Instead, just set the progress property, that's all.

Also, don't call done() from within the doInBackground method as this needs to be called from the EDT by the SwingWorker. So let the SwingWorker itself call this method when it is in fact done.

Also, Done() should be done() -- the first letter shouldn't be capitalized, and you should use @Override annotations in this code so you can be sure that you're overriding methods correctly.

Also, what does this do?

 _model.startSearch(_view.getTerm());

Does it call code that takes a while to complete? Should this be initialized from within the SwingWorker doInBackground itself?

Edit: Another option is to give the Model a bound int property, say called progress, and then add a PropertyChangeListener to it directly letting it update the JProgressBar. For example,

import java.awt.BorderLayout;
import java.awt.event.*;
import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import java.beans.PropertyChangeSupport;

import javax.swing.*;

public class MVC_ProgressBarThread {
   private static void createAndShowUI() {
      MVC_View view = new MVC_View();
      MVC_Model model = new MVC_Model();
      MVC_Control control = new MVC_Control(view, model);
      view.setControl(control);

      JFrame frame = new JFrame("MVC_ProgressBarThread");
      frame.getContentPane().add(view);
      frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
      frame.pack();
      frame.setLocationRelativeTo(null);
      frame.setVisible(true);
   }

   public static void main(String[] args) {
      java.awt.EventQueue.invokeLater(new Runnable() {
         public void run() {
            createAndShowUI();
         }
      });
   }
}

@SuppressWarnings("serial")
class MVC_View extends JPanel {
   private MVC_Control control;
   private JProgressBar progressBar = new JProgressBar();
   private JButton startActionButton = new JButton("Start Action");

   public MVC_View() {
      startActionButton.addActionListener(new ActionListener() {
         public void actionPerformed(ActionEvent e) {
            buttonActionPerformed();
         }
      });

      JPanel buttonPanel = new JPanel();
      buttonPanel.add(startActionButton);
      setLayout(new BorderLayout());
      add(buttonPanel, BorderLayout.NORTH);
      add(progressBar, BorderLayout.CENTER);
   }

   public void setControl(MVC_Control control) {
      this.control = control;
   }

   private void buttonActionPerformed() {
      if (control != null) {
         control.doButtonAction();
      }
   }

   public void setProgress(int progress) {
      progressBar.setValue(progress);
   }

   public void start() {
      startActionButton.setEnabled(false);
   }

   public void done() {
      startActionButton.setEnabled(true);
      setProgress(100);
   }
}

class MVC_Control {
   private MVC_View view;
   private MVC_Model model;

   public MVC_Control(final MVC_View view, final MVC_Model model) {
      this.view = view;
      this.model = model;
      model.addPropertyChangeListener(new PropertyChangeListener() {
         public void propertyChange(PropertyChangeEvent pce) {
            if (MVC_Model.PROGRESS.equals(pce.getPropertyName())) {
               view.setProgress((Integer)pce.getNewValue());
            }
         }
      });
   }

   public void doButtonAction() {
      view.start();
      SwingWorker<Void, Void> swingworker = new SwingWorker<Void, Void>() {
         @Override
         protected Void doInBackground() throws Exception {
            model.reset();
            model.startSearch();
            return null;
         }

         @Override
         protected void done() {
            view.done();
         }
      };
      swingworker.execute();
   }

}

class MVC_Model {
   public static final String PROGRESS = "progress";
   private static final int MAX = 100;
   private static final long SLEEP_DELAY = 100;
   private int progress = 0;
   private PropertyChangeSupport pcs = new PropertyChangeSupport(this);

   public void setProgress(int progress) {
      int oldProgress = this.progress;
      this.progress = progress;

      PropertyChangeEvent evt = new PropertyChangeEvent(this, PROGRESS, oldProgress, progress);
      pcs.firePropertyChange(evt);
   }

   public void reset() {
      setProgress(0);
   }

   public void addPropertyChangeListener(PropertyChangeListener listener) {
      pcs.addPropertyChangeListener(listener);
   }

   public void startSearch() {
      for (int i = 0; i < MAX; i++) {
         int newValue = (100 * i) / MAX;
         setProgress(newValue);
         try {
            Thread.sleep(SLEEP_DELAY);
         } catch (InterruptedException e) {}
      }
   }
}
Hovercraft Full Of Eels
  • 283,665
  • 25
  • 256
  • 373
  • I modified my code as per your suggestions, yes the time heavy code is the startSearch() in the model, view.getTerm() returns the text that the user has entered into a text box. If I put the startsearch into the doinbackground procedure would I need to take the drawing of the progress bar out? – Neutralise Apr 04 '11 at 00:56
  • Again, the doInBackground should have no code that refers to the progress bar or any Swing component. Regardless of anything, take it out of doInBackground. And yeah if the model search code is time heavy, shouldn't it be what belongs in the background thread? That's what you use a SwingWorker for. – Hovercraft Full Of Eels Apr 04 '11 at 00:58
  • I thought that if I called the startSearch in the main thread, the progress bar would update in the new thread. That was my thoughts, but it would seem I am wrong. – Neutralise Apr 04 '11 at 00:59
  • You're locking up the EDT, and again, that's the purpose of using a SwingWorker, to prevent just this from happening. Please read the tutorial [Concurrency in Swing](http://download.oracle.com/javase/tutorial/uiswing/concurrency/) to understand what is going on. – Hovercraft Full Of Eels Apr 04 '11 at 01:00
  • Seems I was premature! It does draw the status bar correctly now, but the actual status does not update. I put a dummy function into my model class: public int getStatus(){ Random r = new Random(); return r.nextInt();}. But the status bar does not update. Would it be easier if I uploaded my new code base so you can see what is going on? – Neutralise Apr 04 '11 at 01:07
  • It will take more effort to fix the problem it seems. I suggest that you create and post an [SSCCE](http://SSCCE.org) in your original post and then post a comment. – Hovercraft Full Of Eels Apr 04 '11 at 01:09
  • Updated. I can't really upload something compilable as this is an assignment and I can't make it public. Sorry. – Neutralise Apr 04 '11 at 01:20
  • your _model.startSearch(getTerm()); is probably blocking. Can the model itself return interim results? – Hovercraft Full Of Eels Apr 04 '11 at 01:32
  • You are correct. I put println before and after calling the models start search and it blocks here until the search is completed. The lock file only exists while the model is searching which means that the loop itself to set the progress bar is never true. Do I need to call the startsearch in another nested thread? Or call it from the main thread? – Neutralise Apr 04 '11 at 01:36
  • If the model has interim results, you can have the model call a public method of the SwingWorker's that calls publish and then set the property in the process method. Or the model can **be** the SwingWorker. – Hovercraft Full Of Eels Apr 04 '11 at 01:44
  • The model searches files from a folder with X files. The search function does this time intensive searching, are you saying I should try and have the model update the swing worker in the view at n intervals? – Neutralise Apr 04 '11 at 01:58
  • @Neutralise: @Hovercraft Full Of Eels is correct. There's a short example [here](http://stackoverflow.com/questions/4637725) that may be helpful. – trashgod Apr 04 '11 at 02:02
  • @trashgod, yes I agree it is the correct method. I am just having problems with my implementation of it. – Neutralise Apr 04 '11 at 02:12
  • By setting my progress bar to intermediate mode, I can see that it is drawing correctly. So now the problem lies in how I get the status from my model. Might need another thread in there also it seems. – Neutralise Apr 04 '11 at 02:16
  • see new suggestion regarding giving model a bound property and listening to it. – Hovercraft Full Of Eels Apr 04 '11 at 03:31