Is MVC in Swing Thread Safe
Asked Answered
O

1

16

I'm trying to touch limits of MVC architecture in Swing, but as I tried everything all (from SwingWorker or Runnable#Thread) are done on EDT

my questions:

  • is there some limits or strictly depends by order of the implementations (wrapped into SwingWorker or Runnable#Thread) ?

  • limited is if is JComponent#method Thread Safe or not ?

  • essential characteristic of an MVC architecture in Swing, ?

  • inc. Container Re-Layout ?

note: for my SSCCE I take one of great examples by HFOE, and maybe by holding this principes strictly isn't possible to create any EDT lack or GUI freeze

enter image description here enter image description here enter image description here enter image description here

import java.awt.BorderLayout;
import java.awt.event.*;
import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import java.beans.PropertyChangeSupport;
import java.util.LinkedList;
import java.util.Queue;
import javax.swing.*;

public class MVC_ProgressBarThread {

    private MVC_ProgressBarThread() {
        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() {

            @Override
            public void run() {
                MVC_ProgressBarThread mVC_ProgressBarThread = new MVC_ProgressBarThread();
            }
        });
    }
}

class MVC_View extends JPanel {

    private static final long serialVersionUID = 1L;
    private MVC_Control control;
    private JProgressBar progressBar = new JProgressBar();
    private JButton startActionButton = new JButton("Press Me and Run this Madness");
    private JLabel myLabel = new JLabel("Nothing Special");

    public MVC_View() {
        startActionButton.addActionListener(new ActionListener() {

            @Override
            public void actionPerformed(ActionEvent e) {
                buttonActionPerformed();
            }
        });
        JPanel buttonPanel = new JPanel();
        startActionButton.setFocusPainted(false);
        buttonPanel.add(startActionButton);
        setLayout(new BorderLayout(10, 10));
        add(buttonPanel, BorderLayout.NORTH);
        progressBar.setStringPainted(true);
        add(progressBar, BorderLayout.CENTER);
        myLabel.setIcon(UIManager.getIcon("OptionPane.questionIcon"));
        myLabel.setHorizontalAlignment(javax.swing.SwingConstants.CENTER);
        add(myLabel, BorderLayout.SOUTH);
    }

    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 setProgressLabel(String label) {
        progressBar.setString(label);
    }

    public void setIconLabel(Icon icon) {
        myLabel.setIcon(icon);
    }

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

    public void done() {
        startActionButton.setEnabled(true);
        setProgress(100);
        setProgressLabel("   Done !!!   ");
        setIconLabel(null);
    }
}

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() {

            @Override
            public void propertyChange(PropertyChangeEvent pce) {
                if (MVC_Model.PROGRESS.equals(pce.getPropertyName())) {
                    view.setProgress((Integer) pce.getNewValue());
                }
                if (MVC_Model.PROGRESS1.equals(pce.getPropertyName())) {
                    view.setProgressLabel((String) pce.getNewValue());
                }
                if (MVC_Model.PROGRESS2.equals(pce.getPropertyName())) {
                    view.setIconLabel((Icon) 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";
    public static final String PROGRESS1 = "progress1";
    public static final String PROGRESS2 = "progress2";
    private static final int MAX = 11;
    private static final long SLEEP_DELAY = 1000;
    private int progress = 0;
    private String label = "Start";
    private PropertyChangeSupport pcs = new PropertyChangeSupport(this);
    private PropertyChangeSupport pcs1 = new PropertyChangeSupport(this);
    private PropertyChangeSupport pcs2 = new PropertyChangeSupport(this);
    private final String[] petStrings = {"Bird", "Cat", "Dog",
        "Rabbit", "Pig", "Fish", "Horse", "Cow", "Bee", "Skunk"};
    private int index = 1;
    private Queue<Icon> iconQueue = new LinkedList<Icon>();
    private Icon icon = (UIManager.getIcon("OptionPane.questionIcon"));

    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 setProgressLabel(String label) {
        String oldString = this.label;
        this.label = label;
        PropertyChangeEvent evt = new PropertyChangeEvent(this, PROGRESS1,
                oldString, label);
        pcs1.firePropertyChange(evt);
    }

    public void setIconLabel(Icon icon) {
        Icon oldIcon = this.icon;
        this.icon = icon;
        PropertyChangeEvent evt = new PropertyChangeEvent(this, PROGRESS2,
                oldIcon, icon);
        pcs2.firePropertyChange(evt);
    }

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

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

    public void startSearch() {
        iconQueue.add(UIManager.getIcon("OptionPane.errorIcon"));
        iconQueue.add(UIManager.getIcon("OptionPane.informationIcon"));
        iconQueue.add(UIManager.getIcon("OptionPane.warningIcon"));
        iconQueue.add(UIManager.getIcon("OptionPane.questionIcon"));
        for (int i = 0; i < MAX; i++) {
            int newValue = (100 * i) / MAX;
            setProgress(newValue);
            setProgressLabel(petStrings[index]);
            index = (index + 1) % petStrings.length;
            setIconLabel(nextIcon());
            try {
                Thread.sleep(SLEEP_DELAY);
            } catch (InterruptedException e) {
            }
        }
    }

    private Icon nextIcon() {
        Icon icon1 = iconQueue.peek();
        iconQueue.add(iconQueue.remove());
        return icon1;
    }
}
Overliberal answered 17/11, 2011 at 15:41 Comment(4)
It's difficult to understand your questions, I'd recommend cleaning it up a little but the short answer is no, Swing by design is not thread safe. I think you already know that so I'm not sure what the actual question is.Depressive
@mKorbel: Your question is not very clear: what does it do that is not correct? How can we see the "lag" that we should see (should we?)? I ran the code and something looks broken (during the first two runs or so I can click the button and then it's grayed out while the progress is running but eventually I can click and launch the progress, yet access the button [Debian/Linux]) but I doubt it's the issue you're complaining about.Cityscape
@mKorbel: you're modifying things on the EDT when you shouldn't. Haven't you got it backward when you write: "as I tried everything all (from SwingWorker or Runnable#Thread) are done on EDT" The point of SwingWorker is to perform lengthy operation outside the EDT. I'm confused by that sentence of yours, need more infos : )Cityscape
+1 for contrasting String[] petStrings with Queue<Icon>. Are you asking why this (intentionally) incorrect program doesn't fail more reliably?Robomb
C
15

This is too long for a comment...

First and this is unrelated to the rest of this answer: there are many different MVCs out there and the one you used in that piece of code you posted here is not the same as the one used in the article you linked to: http://www.oracle.com/technetwork/articles/javase/mvc-136693.html

The article correctly points out that it's just "A common MVC implementation" (one where the view registers a listener listening to model changes). Your implementation is a different type of MVC, where the controller registers a listener listening to model changes and then updates the view.

Not that there's anything wrong with that: there are a lot of different types of MVCs out there (*).

(Another little caveat... Your view is aware of your controller in your example, which is a bit weird: there are other ways to do what you're doing without needing to "feed" the controller to the view like you do with your setControl(...) inside your MVCView.)

But anyway... You're basically nearly always modifying the GUI from outside the EDT (which you shouldn't be doing):

public void setIconLabel(final Icon icon) {
   myLabel.setIcon(icon);
}

You can check it by adding this:

System.out.println("Are we on the EDT? " + SwingUtilities.isEventDispatchThread());

This is because you're eventually doing these updates from your SwingWorker thread (the SwingWorker thread is run outside the EDT: it's basically the point of a Swing worker).

I'd rather update the GUI from the EDT, doing something like this:

public void setIconLabel(final Icon icon) {
    SwingUtilities.invokeLater(new Runnable() {
        @Override
        public void run() {
            myLabel.setIcon(icon);
        }
    });
}
Cityscape answered 17/11, 2011 at 16:34 Comment(3)
(*) HVMC/PAC being my favorite(s) type(s) of MVC but I digress ; )Cityscape
+1 for answer, 1) agreed that there are another MVC models, I follows better that I saw around (by Swing Guru - HFOE), 2) as I said - but as I tried everything all are done on EDT, 3) from Java1.6.021 is SwingWorker more stable but (all E.T. bugs are still in BugsDB bugs.sun.com/bugdatabase/view_bug.do?bug_id=6826514 and another), and if you'll override #7054365 , then output from done() is always in EDT, then there no reason for test that :-)Overliberal
@user988052 hmmm Substance L&F required wrapping output to the invokeLater,Overliberal

© 2022 - 2024 — McMap. All rights reserved.