According to MVC, where does this code belong?

Question

Consider the following GUI Screen:

On the left, there is the Person List and on the right, there is the Person. Whenever the selecton of person list changes, the selected person (firstname and lastname) must be shown in the right.

My question is, where the following code belongs? What would be more "MVC"?

    personListModel.addPropertyChangeListener("selection", e -> {
        personModel.setFromDomainEntity(personListModel.getSelectedPerson().orElse(null));
    });

---

The full example is this:

public class MvcExample {
    public static void main(String[] args) {
        SwingUtilities.invokeLater(() -> {

            JFrame frame = new JFrame("Example");
            frame.setLayout(new BorderLayout());
            frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

            PersonListModel personListModel = new PersonListModel();
            PersonListView listView = new PersonListView(personListModel);

            frame.add(listView, BorderLayout.LINE_START);

            PersonModel personModel = new PersonModel();
            PersonView personView = new PersonView(personModel);

            frame.add(personView, BorderLayout.CENTER);

            personListModel.addPropertyChangeListener("selection", e -> {
                personModel.setFromDomainEntity(personListModel.getSelectedPerson().orElse(null));
            });

            frame.pack();
            frame.setLocationByPlatform(true);
            frame.setVisible(true);
        });
    }

    static class PersonModel {
        private String firstName, lastName;
        private SwingPropertyChangeSupport listeners;

        public PersonModel() {
            this.firstName = "";
            this.lastName = "";
            listeners = new SwingPropertyChangeSupport(this);
        }

        public void setFirstName(String firstName) {
            if (firstName.equals(this.firstName))
                return;

            this.firstName = firstName;
            listeners.firePropertyChange("firstname", null, null);
        }

        public void setLastName(String lastName) {
            if (lastName.equals(this.lastName))
                return;

            this.lastName = lastName;
            listeners.firePropertyChange("lastname", null, null);
        }

        public String getFirstName() {
            return firstName;
        }

        public String getLastName() {
            return lastName;
        }

        public void setFromDomainEntity(Person person) {
            setFirstName(person == null ? "" : person.firstName);
            setLastName(person == null ? "" : person.lastName);
        }

        void addPropertyChangeListener(String property, PropertyChangeListener listener) {
            listeners.addPropertyChangeListener(property, listener);
        }
    }

    static class PersonView extends JPanel {
        private PersonModel model;
        private JTextField firstNameField = new JTextField(15);
        private JTextField lastNameField = new JTextField(15);

        public PersonView(PersonModel model) {
            super(new FlowLayout());
            setBorder(BorderFactory.createTitledBorder("Person View / Person Model"));
            this.model = model;

            firstNameField.setText(model.getFirstName());
            lastNameField.setText(model.getLastName());

            firstNameField.getDocument().addDocumentListener(new RunnableDocumentListener(() -> {
                if (!model.getFirstName().equals(firstNameField.getText()))
                    model.setFirstName(firstNameField.getText());
            }));

            lastNameField.getDocument().addDocumentListener(new RunnableDocumentListener(() -> {
                if (!model.getLastName().equals(lastNameField.getText()))
                    model.setLastName(lastNameField.getText());
            }));

            model.addPropertyChangeListener("firstname", e -> {
                if (firstNameField.getText().equals(model.getFirstName()))
                    return;

                firstNameField.setText(model.getFirstName());
            });

            model.addPropertyChangeListener("lastname", e -> {
                if (lastNameField.getText().equals(model.getLastName()))
                    return;

                lastNameField.setText(model.getLastName());
            });

            add(firstNameField);
            add(lastNameField);
        }

        //@formatter:off
        private static class RunnableDocumentListener implements DocumentListener{
            private Runnable r;
            public RunnableDocumentListener(Runnable r) {this.r = r;}
            @Override
            public void insertUpdate(DocumentEvent e) { this.r.run();}

            @Override
            public void removeUpdate(DocumentEvent e) { this.r.run();}

            @Override
            public void changedUpdate(DocumentEvent e) {this.r.run();}
        }
        //@formatter:on
    }

    static class PersonListView extends JPanel {

        private JList<Person> personList;
        private DefaultListModel<Person> listModel;
        private PersonListModel model;

        public PersonListView(PersonListModel model) {
            super(new BorderLayout());
            setPreferredSize(new Dimension(400, 400));
            setBorder(BorderFactory.createTitledBorder("Person List View / Person List Model"));
            this.model = model;

            listModel = new DefaultListModel<>();
            personList = new JList<>(listModel);
            personList.setCellRenderer(new DefaultListCellRenderer() {
                @Override
                public Component getListCellRendererComponent(JList<?> list, Object value, int index,
                        boolean isSelected, boolean cellHasFocus) {
                    JLabel renderer = (JLabel) super.getListCellRendererComponent(list, value, index, isSelected,
                            cellHasFocus);
                    Person person = (Person) value;
                    renderer.setText(person.firstName + " - " + person.lastName);
                    return renderer;
                }
            });

            syncData();
            syncSelection();

            model.addPropertyChangeListener("data", e -> {
                syncData();
            });

            model.addPropertyChangeListener("selection", e -> {
                syncSelection();
            });

            personList.getSelectionModel().addListSelectionListener(e -> {
                model.setSelectedPerson(personList.getSelectedValue());
            });

            add(new JScrollPane(personList));
        }

        private void syncData() {
            listModel.removeAllElements();
            for (Person p : model.getPersons()) {
                listModel.addElement(p);
            }
        }

        private void syncSelection() {
            if (personList.getSelectedValue() == model.getSelectedPerson().orElse(null))
                return;

            personList.setSelectedValue(model.getSelectedPerson(), true);
        }
    }

    // Domain Entity
    static class Person {
        private String firstName, lastName;

        public Person(String firstName, String lastName) {
            this.firstName = firstName;
            this.lastName = lastName;
        }
    }

    static class PersonListModel {
        private List<Person> persons;
        private Person selectedPerson;
        private SwingPropertyChangeSupport listeners;

        public PersonListModel() {
            persons = new ArrayList<>();

            persons.add(new Person("Stack", "OverFlow"));
            persons.add(new Person("Jackie", "Chan"));
            persons.add(new Person("Something", "Else"));

            listeners = new SwingPropertyChangeSupport(this);
        }

        public void setSelectedPerson(Person selectedPerson) {
            if (this.selectedPerson == selectedPerson)
                return;

            this.selectedPerson = selectedPerson;
            listeners.firePropertyChange("selection", null, null);
        }

        public Optional<Person> getSelectedPerson() {
            return Optional.ofNullable(selectedPerson);
        }

        public List<Person> getPersons() {
            return Collections.unmodifiableList(persons);
        }

        void addPropertyChangeListener(String property, PropertyChangeListener listener) {
            listeners.addPropertyChangeListener(property, listener);
        }
    }
}

My thoughts are that there 4 places that I can put it.

Option #1: As in the example. The parent VC part holds it. There is an ApplicationFrame that extends a JFrame and depends on a PersonListView and on a PersonView. And it adds the coordination between the 2 models:

class ApplicationFrame extends JFrame {
    public ApplicationFrame(PersonListView personListView, PersonView personView) {
        //...
        personListView.getModel().addPropertyChangeListener("selection",e->{
            personView.getModel().setFromDomainEntity(personListModel.getSelectedPerson().orElse(null));
        });
        
        //...
    }
}

This approach seems to be approriate, but if there are more models that need to be coordinated like that, ApplicationFrame will end up with too much code and too many reposibilities.

Option #2 One model depends on the other and makes the call explicit (or implicit):

public PersonListModel(PersonModel personModel) {
    //...
}

public void setSelectedPerson(Person selectedPerson) {
    if (this.selectedPerson == selectedPerson)
        return;

    this.selectedPerson = selectedPerson;
    personModel.setFromDomainEntity(selectedPerson);
    listeners.firePropertyChange("selection", null, null);
}

or:

public PersonModel(PersonListModel listToObserve) {
    //
    listToObserve.addPropertyChangeListener("selection", e->{
        setFromDomainEntity(listToObserve.getSelectedPerson().orElse(null));
    });
}

Now, if more models get involved, they all get too complicated. Which impacts testability as well.

Option #3: Respect model hierarchy as the views go. PersonView and PersonListView are added on a (say) MainView. So there will be likely a MainModel. And this MainModel depends on PersonModel and PersonListModel. So the code goes there. This will lead the project in a big hierarchy, and I think it will be difficult to debug, or test that. Plus, what will this MainModel end up being if more models get involved?

Option #4: Introduce a new class. Say something like "PersonListToPersonCoordinator" that stands there to coordinate between the 2 models. And if another model needs to "hear" the selected Person on the list, a new class is introduced "PersonListToTheOtherModelCoordinator" that just does the same. This sounds the best option for me till now. The complex/long hierarchies are absent and the testability is there since there are no model dependences between models.

But what does MVC has to say about that?

Answer 1

I replicated your GUI. I still have no idea what you mean by the word "observe". All observing in Swing happens under the covers. You don't have to deal with observing at all.

When you left-click on a name in the JList, the name is copied to the form.

I created a Person class and a PersonsModel class. Both classes are plain Java getter / setter classes. The PersonsModel class is a List of Person instances.

It turned out that I could copy the List of Person instances into a DefaultListModel with one method call.

Person and PersonsModel are my model classes. JListPersonGUI is my view class. PersonSelectionListener is my controller class. My controller class didn't even need to reference the model, since I copied the model into the DefaultListModel.

Here's the complete runnable code. I made all the classes inner classes so I could post the code as one block.

import java.awt.BorderLayout;
import java.awt.Dimension;
import java.awt.FlowLayout;
import java.util.ArrayList;
import java.util.List;

import javax.swing.BorderFactory;
import javax.swing.DefaultListModel;
import javax.swing.DefaultListSelectionModel;
import javax.swing.JFrame;
import javax.swing.JList;
import javax.swing.JPanel;
import javax.swing.JScrollPane;
import javax.swing.JTextField;
import javax.swing.SwingUtilities;
import javax.swing.event.ListSelectionEvent;
import javax.swing.event.ListSelectionListener;

public class JListPersonGUI implements Runnable {

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new JListPersonGUI());
    }
    
    private JList<Person> personList;
    
    private JTextField firstNameField;
    private JTextField lastNameField;
    
    private final PersonsModel model;
    
    public JListPersonGUI() {
        this.model = new PersonsModel();
    }

    @Override
    public void run() {
        JFrame frame = new JFrame("Example");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        
        frame.add(createJListPanel(), BorderLayout.BEFORE_LINE_BEGINS);
        frame.add(createPersonPanel(), BorderLayout.AFTER_LINE_ENDS);
        
        frame.pack();
        frame.setLocationByPlatform(true);
        frame.setVisible(true);
    }
    
    private JPanel createJListPanel() {
        JPanel panel = new JPanel(new BorderLayout());
        panel.setPreferredSize(new Dimension(400, 400));
        panel.setBorder(BorderFactory.createTitledBorder(
                "Person List View / Person List Model"));

        DefaultListModel<Person> listModel = new DefaultListModel<>();
        listModel.addAll(model.getPersons());
        personList = new JList<>(listModel);
        personList.addListSelectionListener(new PersonSelectionListener(this));
        personList.setSelectionMode(DefaultListSelectionModel.SINGLE_SELECTION);
        JScrollPane scrollPane = new JScrollPane(personList);
        panel.add(scrollPane, BorderLayout.CENTER);
        
        return panel;
    }
    
    private JPanel createPersonPanel() {
        JPanel panel = new JPanel(new FlowLayout());
        panel.setBorder(BorderFactory.createTitledBorder(
                "Person View / Person Model"));
        
        firstNameField = new JTextField(15);
        panel.add(firstNameField);
        
        lastNameField = new JTextField(15);
        panel.add(lastNameField);
        
        return panel;
    }
    
    public void updatePersonPanel(Person person) {
        firstNameField.setText(person.getFirstName());
        lastNameField.setText(person.getLastName());
    }
    
    public JList<Person> getPersonList() {
        return personList;
    }

    public class PersonSelectionListener implements ListSelectionListener {

        private final JListPersonGUI view;
        
        public PersonSelectionListener(JListPersonGUI view) {
            this.view = view;
        }

        @Override
        public void valueChanged(ListSelectionEvent event) {
            Person person = view.getPersonList().getSelectedValue();
            view.updatePersonPanel(person);
        }
        
    }
    
    public class PersonsModel {
        
        private final List<Person> persons;
        
        public PersonsModel() {
            this.persons = new ArrayList<>();
            this.persons.add(new Person("John", "Smith"));
            this.persons.add(new Person("Sally", "Andrews"));
            this.persons.add(new Person("Charles", "Barkley"));
        }

        public List<Person> getPersons() {
            return persons;
        }
        
    }
    
    public class Person {
        private final String firstName, lastName;

        public Person(String firstName, String lastName) {
            this.firstName = firstName;
            this.lastName = lastName;
        }

        public String getFirstName() {
            return firstName;
        }

        public String getLastName() {
            return lastName;
        }
        
        @Override
        public String toString() {
            return firstName + " - " + lastName;
        }
        
    }

}

Edited to add: The question is, "shouldn't the view observe (observer pattern) the model for changes?"

The answer in Swing is no. In a GUI, changes happen as a result of some sort of action. A button click, a menu selection, a JList selection. Each change, in Swing, triggers some sort of listener. The listener is a controller. The listener handles changes to the model.

Now, in a generalized Java application, you can do the same thing. You can create action listeners as a response to triggers. It's hard for me to answer this in the abstract. For instance, you can trigger a database read every 15 minutes to check for updates.

I just finished modifying your GUI to handle more actions. Again, no observers are necessary.

Here's the complete runnable code. I modified the model classes, the view class, and added one controller class for the JButtons.

import java.awt.BorderLayout;
import java.awt.Dimension;
import java.awt.FlowLayout;
import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.Insets;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.ArrayList;
import java.util.List;

import javax.swing.BorderFactory;
import javax.swing.DefaultListModel;
import javax.swing.DefaultListSelectionModel;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JList;
import javax.swing.JPanel;
import javax.swing.JScrollPane;
import javax.swing.JTextField;
import javax.swing.SwingUtilities;
import javax.swing.event.ListSelectionEvent;
import javax.swing.event.ListSelectionListener;

public class JListPersonGUI implements Runnable {

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new JListPersonGUI());
    }
    
    private JList<Person> personList;
    
    private JTextField firstNameField;
    private JTextField lastNameField;
    
    private Person selectedPerson;
    
    private final PersonSelectionListener personSelectionListener;
    
    private final PersonsModel model;
    
    public JListPersonGUI() {
        this.model = new PersonsModel();
        this.personSelectionListener = new PersonSelectionListener(this);
    }

    @Override
    public void run() {
        JFrame frame = new JFrame("Example");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        
        frame.add(createJListPanel(), BorderLayout.BEFORE_LINE_BEGINS);
        frame.add(createPersonPanel(), BorderLayout.AFTER_LINE_ENDS);
        
        frame.pack();
        frame.setLocationByPlatform(true);
        frame.setVisible(true);
    }
    
    private JPanel createJListPanel() {
        JPanel panel = new JPanel(new BorderLayout());
        panel.setPreferredSize(new Dimension(400, 400));
        panel.setBorder(BorderFactory.createTitledBorder(
                "Person List View / Person List Model"));

        DefaultListModel<Person> listModel = new DefaultListModel<>();
        listModel.addAll(model.getPersons());
        personList = new JList<>(listModel);
        personList.addListSelectionListener(personSelectionListener);
        personList.setSelectionMode(DefaultListSelectionModel.SINGLE_SELECTION);
        JScrollPane scrollPane = new JScrollPane(personList);
        panel.add(scrollPane, BorderLayout.CENTER);
        
        return panel;
    }
    
    private JPanel createPersonPanel() {
        JPanel panel = new JPanel(new FlowLayout());
        panel.setBorder(BorderFactory.createTitledBorder(
                "Person View / Person Model"));
        
        JPanel gridPanel = new JPanel(new GridBagLayout());
        
        GridBagConstraints gbc = new GridBagConstraints();
        gbc.anchor = GridBagConstraints.LINE_START;
        gbc.fill = GridBagConstraints.HORIZONTAL;
        gbc.insets = new Insets(5, 5, 5, 5);
        gbc.gridwidth = 1;
        gbc.weightx = 1.0;
        
        gbc.gridx = 0;
        gbc.gridy = 0;
        JLabel label = new JLabel("First Name:");
        gridPanel.add(label, gbc);
        
        gbc.gridx++;
        firstNameField = new JTextField(15);
        gridPanel.add(firstNameField, gbc);
        
        gbc.gridx = 0;
        gbc.gridy++;
        label = new JLabel("Last Name:");
        gridPanel.add(label, gbc);
        
        gbc.gridx++;
        lastNameField = new JTextField(15);
        gridPanel.add(lastNameField, gbc);
        
        JPanel buttonPanel = new JPanel(new FlowLayout(FlowLayout.LEADING, 5, 5));
        
        ButtonListener listener = new ButtonListener(this, model);
        
        JButton addButton = new JButton("Add Name");
        addButton.addActionListener(listener);
        buttonPanel.add(addButton);
        
        JButton updateButton = new JButton("Update Name");
        updateButton.addActionListener(listener);
        buttonPanel.add(updateButton);
        
        JButton deleteButton = new JButton("Delete Name");
        deleteButton.addActionListener(listener);
        buttonPanel.add(deleteButton);
        
        Dimension ad = addButton.getPreferredSize();
        Dimension ud = updateButton.getPreferredSize();
        Dimension dd = deleteButton.getPreferredSize();
        Dimension ld = calculateLargestDimension(ad, ud, dd);
        addButton.setPreferredSize(ld);
        updateButton.setPreferredSize(ld);
        deleteButton.setPreferredSize(ld);
        
        gbc.gridwidth = 2;
        gbc.gridx = 0;
        gbc.gridy++;
        gridPanel.add(buttonPanel, gbc);
        
        panel.add(gridPanel);
        
        return panel;
    }
    
    private Dimension calculateLargestDimension(Dimension... dimensions) {
        int width = 0;
        int height = 0;
        
        for (int index = 0; index < dimensions.length; index++) {
            width = Math.max(width, dimensions[index].width + 10);
            height = Math.max(height, dimensions[index].height);
        }
        
        return new Dimension(width, height);
    }
    
    public void updatePersonPanel(Person person) {
        this.selectedPerson = person;
        firstNameField.setText(person.getFirstName());
        lastNameField.setText(person.getLastName());
    }
    
    public String getFirstName() {
        return firstNameField.getText().trim();
    }
    
    public String getLastName() {
        return lastNameField.getText().trim();
    }
    
    public Person getSelectedPerson() {
        return selectedPerson;
    }

    public PersonSelectionListener getPersonSelectionListener() {
        return personSelectionListener;
    }

    public JList<Person> getPersonList() {
        return personList;
    }

    public class PersonSelectionListener implements ListSelectionListener {

        private final JListPersonGUI view;
        
        public PersonSelectionListener(JListPersonGUI view) {
            this.view = view;
        }

        @Override
        public void valueChanged(ListSelectionEvent event) {
            Person person = view.getPersonList().getSelectedValue();
            view.updatePersonPanel(person);
        }
        
    }
    
    public class ButtonListener implements ActionListener {
        
        private final JListPersonGUI view;
        
        private final PersonsModel model;

        public ButtonListener(JListPersonGUI view, PersonsModel model) {
            this.view = view;
            this.model = model;
        }

        @Override
        public void actionPerformed(ActionEvent event) {
            String action = event.getActionCommand();
            Person person = view.getSelectedPerson();
            
            switch (action) {
            case "Add Name":
                model.addPerson(new Person(view.getFirstName(), view.getLastName()));
                break;
            case "Update Name":
                int index = model.getPersonIndex(person);
                if (index >= 0) {
                    Person updatePerson = model.getPersons().get(index);
                    updatePerson.setFirstName(view.getFirstName());
                    updatePerson.setLastName(view.getLastName());
                }
                break;
            case "Delete Name":
                model.removePerson(person);
                break;
            }
            
            personList.removeListSelectionListener(view.getPersonSelectionListener());
            DefaultListModel<Person> listModel = new DefaultListModel<>();
            listModel.addAll(model.getPersons());
            view.getPersonList().setModel(listModel);
            personList.addListSelectionListener(view.getPersonSelectionListener());
        }
        
    }
    
    public class PersonsModel {
        
        private final List<Person> persons;
        
        public PersonsModel() {
            this.persons = new ArrayList<>();
            this.persons.add(new Person("John", "Smith"));
            this.persons.add(new Person("Sally", "Andrews"));
            this.persons.add(new Person("Charles", "Barkley"));
        }
        
        public void addPerson(Person person) {
            this.persons.add(person);
        }
        
        public void removePerson(Person person) {
            this.persons.remove(person);
        }
        
        public int getPersonIndex(Person person) {
            for (int index = 0; index < persons.size(); index++) {
                Person listPerson = persons.get(index);
                if (listPerson.getFirstName().equals(person.getFirstName()) 
                        && listPerson.getLastName().equals(person.getLastName())) {
                    return index;
                }
            }
            
            return -1;
        }

        public List<Person> getPersons() {
            return persons;
        }
        
    }
    
    public class Person {
        private String firstName, lastName;

        public Person(String firstName, String lastName) {
            this.firstName = firstName;
            this.lastName = lastName;
        }

        public void setFirstName(String firstName) {
            this.firstName = firstName;
        }

        public void setLastName(String lastName) {
            this.lastName = lastName;
        }

        public String getFirstName() {
            return firstName;
        }

        public String getLastName() {
            return lastName;
        }
        
        @Override
        public String toString() {
            return firstName + " - " + lastName;
        }
        
    }

}