Simple Java MVC (swing)

Question

I'm a Java beginner currently practising MVC pattern and came up with this. Could you please check if this is a proper implementation of MVC and if there are any best practices that I have broken?

public class Appka { public static void main(String[] args) { Model model = new Model(); Controller controller = new Controller(model); View view = new View(model, controller); model.addListener(view); } } public class Model { String value; List<StateChangedListener> listeners = new ArrayList<>(); public Model() { this.value = ""; } public String getValue() { return value; } public void setValue(String value) { this.value = value; notifyListeners(); } public void addListener(StateChangedListener l) { listeners.add(l); } public void removeListener(StateChangedListener l) { listeners.remove(l); } public void notifyListeners() { listeners.forEach(l -> l.stateChanged()); } } public class View extends JFrame implements StateChangedListener { private Model model; private Controller controller; private JLabel label1; private JLabel label2; private JButton button1; private JButton button2; public View(Model model, Controller controller) { this.model = model; this.controller = controller; initComponents(); } private void initComponents() { button1 = new JButton(); button2 = new JButton(); label1 = new JLabel(); label2 = new JLabel(); setDefaultCloseOperation(javax.swing.WindowConstants.EXIT_ON_CLOSE); button1.setText("1"); button1.addActionListener(controller); button2.setText("2"); button2.addActionListener(controller); label1.setText("label1"); label2.setText("label2"); javax.swing.GroupLayout layout = new javax.swing.GroupLayout(getContentPane()); getContentPane().setLayout(layout); layout.setHorizontalGroup( layout.createParallelGroup(javax.swing.GroupLayout.Alignment.LEADING) .addGroup(layout.createSequentialGroup() .addContainerGap() .addGroup(layout.createParallelGroup(javax.swing.GroupLayout.Alignment.LEADING) .addComponent(button1) .addComponent(label1)) .addGap(26, 26, 26) .addGroup(layout.createParallelGroup(javax.swing.GroupLayout.Alignment.LEADING) .addComponent(label2) .addComponent(button2)) .addContainerGap(113, Short.MAX_VALUE)) ); layout.setVerticalGroup( layout.createParallelGroup(javax.swing.GroupLayout.Alignment.LEADING) .addGroup(layout.createSequentialGroup() .addGap(24, 24, 24) .addGroup(layout.createParallelGroup(javax.swing.GroupLayout.Alignment.BASELINE) .addComponent(label1) .addComponent(label2)) .addGap(18, 18, 18) .addGroup(layout.createParallelGroup(javax.swing.GroupLayout.Alignment.BASELINE) .addComponent(button1) .addComponent(button2)) .addContainerGap(30, Short.MAX_VALUE)) ); pack(); setVisible(true); } @Override public void stateChanged() { label1.setText(model.getValue()); label2.setText(model.getValue()); } } public class Controller implements ActionListener { private Model model; public Controller(Model model) { this.model = model; } @Override public void actionPerformed(ActionEvent e) { JButton clicked = (JButton) e.getSource(); model.setValue(clicked.getText()); } } interface StateChangedListener { public void stateChanged(); } 

Answer 1

(this answer is advice and not a real code review, but I can't comment)

I discourage to use swing for your GUI. Java is replacing swing with JavaFX. But that's not the only reason why I should use JavaFX. The most important reason is that with JavaFX it is easier to follow the MVC pattern.

A very good JavaFx and MVC tutorial I really recommend and wich shows the whole worflow and structure: link

Answer 2

There are a couple of problems:

1. You connected the buttons instead of the text fields to your model.

2. You have two text fields but only a single value in your model. In proper MVC, each text field should be attached to a different property of the model.

3. You always send a change event. Your model should only send change events when the new value isn't the same as the old one.

4. You should tell your StateChangedListener which aspect of the model changed. Pass the name of the property, for example.