Is there something wrong with Swing's MVC implementation for JList?

Question

Some time ago I asked this question. All solutions are workarounds.

Now this can't be. I feel that something is wrong here, but I can't tell if it is Swing's MVC model that is conceptually wrong, or if it is my thinking that is conceptually wrong.

Here is the problem again. I am using a JList to implement a list of thumbnails for the pages of a document. If the user selects another thumbnail from the list, that page is loaded. To do this I added a ListSelectionListener to the JList, which when the selection changes, it loads that page. But the user can also change the page using another control. Naturally, I want this to be reflected in the thumbnail list by having that page selected here. So I setSelectedIndex() to update the JList. Unfortunately this has the unwanted effect of raising a ListSelectionEvent which causes the listener to reload the page.

Now what is wrong here? I just changed the model from somewhere else, so naturally I want the view to update itself, but I don't want it to trigger events. Is Swing not implementing MVC right? Or am I missing a point here?

Statics

Thanks everybody for the great answers! I accepted @britishmutt's answer, because it's the most detailed and insightful and it contains the cleanest solution. The problem is that the component that loads the page should see that it was requested to load the same page and should not do it. The links were very useful. I still think Swing's MVC model is defective. They should have gone the traditional way. Their model seems to be more trouble than it's worth. – Statics

I have encountered the same problem with listener updates several times. Imagine if you have N components that have to update each other... Even if you check for a real display change to decide whether or not to fire an event, there will be N-1 events fired from the other N-1 components after they get updated. – Hysterectomize

Answer 1

This is an issue that a lot of us Swing programmers have to face: multiple controls modifying the same data, with the update then reflected in each control. At some point, something has to have the ultimate veto on what updates will be applied to the model: whatever that something is needs to be able to handle multiple (potentially redundant or even contradictory) updates and decide what to do with them. This could happen in the model layer, but ideally it should be the controller that does this - this piece, after all, is most likely where the business logic resides.

The problem with Swing, in this regard, is that the controller piece of MVC is often split somewhat between the view component and the model so it can be difficult to have that logic centralised. To some extent the Action interface rectifies this for actionPerformed() events by putting the logic in one place and allowing it to be shared by different components, but this doesn't help for the other types of event, or when there are multiple different classes of event that need to be coordinated.

The answer, then, is to follow a pattern that's hinted at in Swing but not made explicit: only perform the requested update if the state will actually change, otherwise do nothing. An example of this is in the JList itself: if you attempt to set the selected index of a JList to the same index that's already selected nothing will happen. No events will be fired, no updates will occur: the update request is effectively ignored. This is a good thing. This means that you can, for example, have a listener on your JList that will respond to a newly selected item, and then in turn ask the same JList to reselect that same item and you won't get stuck in a pathologically recursive loop. If all the model-controllers in an application do this then there's no problem with multiple, repeated events firing off all over the place - each component will only update itself (and subsequently fire off events) if it needs to, and if it does update then it can fire off all the update events it wants, but only those components that haven't already got the message will do anything about it.

Possessed

I appreciate the detailed answer; it gives a lot of insight. I was afraid I was the only one experiencing this problem, and it was bugging me because it has appeared a couple of times again. What you suggest in the last paragraph is what was happening at first. But while it didn't go into an infinite loop, it still loaded the page twice. I also don't like all these events flying around all over the place. At least I'm glad it's a general problem, and I feel less stupid now. – Statics

Answer 2

This is expected behaviour.

From Model-View-Controller [Wikipedia]:

In event-driven systems, the model notifies observers (usually views) when the information changes so that they can react.

So, when you call setSelectedIndex on the JList, you are updating its model, which then notifies each ListSelectionListener. It wouldn't be MVC if you could "silently" update a model without letting anyone know.

Pain

Great quote! You are absolutely right. But now that I think of it, the JList is a View, so it has to be updated. But the ListSelectionListener isn't a view. It's a controller. So I think this is the problem. Like @OscarRyz said, the controller and the view are closely related. Actually, the controller is coupled to the model. Strange! Because the controller should listen to user actions (like clicking on another item), not changes in the model. So I think that is the problem. Instead of addListSelectionListener(), there should be a addItemClickedListener() of something like that. – Statics

Answer 3

Swing is not exactly MVC, but has it's roots in MVC ( the difference lays in the fact, the view and the controller in Swing are more closely related than in other MVC see Swing architecture for more details ).

But this may not seem to be the problem you're facing. It sounds like you have to validate in the event listeners, for the type of event and decide whether to ignore it or not: If the event was originated in the list do change it. If was triggered by some other control, do not.

Hugh

I don't think I can check where it originated from. The event is created by the JList (or by its model to be more exact), but this is the same if the user clicks on an item. The event does not have any information of anything before that. – Statics

It's not where it comes from that matters, it's whether or not the event is going to cause a change in state of the model and hence the display. Whatever piece of code is ultimately responsible for refreshing the display of the document needs to know what document is currently being displayed, and if it is asked to display the same document again it should ignore the event. – Possessed

Answer 4

What @dogbane said.

But to fix the problem you need to add some sort of a state check during the listener to see if the event is one you should ignore. The ListSelectionEvent has a getValueAdjusting() method, but that is mostly internal. What you need to do is simulate it yourself.

So for example when you update the list from an external selection you would have code like...

try {
    setSelectionAdjusting(true);
    /* ... your old update code ... */
} finally {
    setSelectionAdjusting(false);
}

and in the update code for the ListSelectionListenerEvent

public void valueChanged(ListSelectionEvent e) {
    if (!isSelectionAdjusting()) {
        /* ... do what you did before ...*/
    }
}

Scoping and access issues are left as an exercise for the reader. You would have to write the setSelectionAdjusting, and possibly set it on other objects as well.

Popular

Sounds like another hack to me. I still feel like there is something conceptually wrong here. – Statics

Answer 5

I still feel like there is something conceptually wrong here.

I empathize, but it may help to consider that you don't have a simple JList observing a ListSelectionModel. Instead, you have a JList and some-other-control observing a hybrid selection model. In @Taisin's example, the hybrid is a CustomSelectionModel that extends DefaultListSelectionModel and allows silent changes. When compatible, it's also possible to share a model, as suggested in this question & answer and this SharedModelDemo from the tutorial.

For reference, this thread cites the article Java SE Application Design With MVC: Issues With Application Design, which addresses the issue in more detail.

Cora

Thanks for the empathy! Yes, I think this seems the cleanest solution. But if you think about it it's like you're solving that conceptual error in the Swing model. In the article you mention, this is step 3: The model is updated. It notifies the controller of its property change. This is not MVC. The model doesn't ever notify the controller. It notifies the view. By overwriting the model to not throw updates, you effectively fix this bug that is present at step 3. – Statics

Your're right, it not pure MVC; it's a separable model architecture, cited by @OscarRyz: java.sun.com/products/jfc/tsc/articles/architecture/#separable – Cora

And even clearer here I think: oracle.com/technetwork/articles/javase/index-142890.html#3 I don't know why they chose this design. The reason they say is " Using this modified MVC helps to more completely decouple the model from the view." But why is this good? What problem does it solve? At the moment, to me it seems more of a hassle. – Statics

Answer 6

I always do like this:

    public class MyDialog extends JDialog {
        private boolean silentGUIChange = false;

    public void updateGUI {
        try {
            silenGUIChange = true;

            // DO GUI-Updates here:
            textField.setText("...");
            checkBox.setSelected (...);

        }
        finally {
            silentGUIChange = false;
        }
    }

    private void addListeners () {
        checkBox.addChangeListener (new ChangeListener () {
           public void stateChanged (ChangeEvent e) {
              if (silentGUIChange)
                 return;

              // update MODEL
              model.setValue(checkBox.isSelected());
          }
         });
    }

}

Audiophile

Answer 7

Typically the way a listener works is it will "go off" every time the event that it is waiting for occurs. If I had to speculate it is a misunderstanding from your side of things.

Arabinose