Intercommunication Between Cards

I got some really good responses here last time, which really helped out quite a bit, so I thought I'd try again with a new batch.

Below is the second phase of my first Java project: Intercommunication between panels. This builds upon the old code, though it has been heavily modified since the first version. Many of the suggestions from the previous question have been implemented and a lot of new code and features have been added. In fact most of this is new code and does not concern the previous question. I only mention this in case you helped in the previous question or were curious. You may view the previous question if you need more context. As before, specific questions and comments are located below the code.

JFileParser.java(main)

package my;

import my.controllers.Logger;

import my.views.Deck;
import my.controllers.DeckNavigator;

import javax.swing.JFrame;
import java.awt.BorderLayout;

import javax.swing.SwingUtilities;

public class JFileParser implements Runnable {

    public static void main( String[] args ) {
        SwingUtilities.invokeLater( new JFileParser() );
    }

    @Override
    public void run() {
        JFrame frame = new JFrame( "Java File Parser" );

        Deck deck = new Deck();
        DeckNavigator navigator = new DeckNavigator();

        navigator.setDeck( deck );
        deck.setNavigator( navigator );
        deck.initDeck();

        frame.getContentPane().add( deck, BorderLayout.NORTH );
        frame.getContentPane().add( navigator, BorderLayout.SOUTH );

        frame.pack();
        frame.setLocationRelativeTo( null );
        frame.setDefaultCloseOperation( JFrame.EXIT_ON_CLOSE );

        frame.setVisible( true );
    }
}

Deck.java

```
package my.views;

import my.controllers.DeckNavigator;

import javax.swing.JPanel;
import java.awt.CardLayout;

import java.util.List;
import java.util.Arrays;

public class Deck extends JPanel {

private CardLayout layout;
private DeckNavigator navigator;

priv

I have a few notes:

• The reason the this keyword doesn't work in main is because main is a static method. The this keyword refers to the current object, and in static methods, there is no current object. Static methods are associated with the class, not the instance. See Using the this keyword, Understanding instance and class members

• Your DeckNavigator is not a controller. It's another view. A controller usually implements a listener and communicates with both the model and the view. A view is usually represented by a JComponent subclass. (Note: JPanel subclasses are JComponent subclasses.) With such a simple program, I would leave out MVC and put everything in the view. For something more complex, you'd need a model to handle the state changes and a real controller. See GUI Architectures

• You should probably combine your two views into one and make that either the content pane or put it in the center of the content pane. Have two subpanels in that panel to replace the Deck and DeckNavigator. It will aid in communication between the panels because they will be in the same class.

• You shouldn't need to make the names of the subpanels of deck constant and then put them in a list, especially considering you only use them for the card layout. Make the list constant, and get the names exclusively from the list.

-
I don't think you need a HashMap here. A list is sufficient. I would store the current index in an instance variable, that way you don't need to call indexOf which can be costly (O(n)). Then you can use a trick like this

private void prevIndex() {
    // Note: You cannot decrement the value of curIndex because curIndex
    // could become negative and modding a negative number gives
    // unexpected results. See http://www.velocityreviews.com/forums/t3883
    // 45-mod-of-a-negative-number.html
    curIndex = (curIndex + CARD_NAMES.size() - 1) % CARD_NAMES.size();
}

private void nextIndex() {
    curIndex = (curIndex + 1) % CARD_NAMES.size();
}

Edit addressing comments:

-
To be able to change the title of your window, you would need to pass the JFrame into the Controller. The controller should be the ActionListener for the buttons and the Model should hold the list of strings. The Controller gets the appropriate string from the Model and sets the title of the window. Something like this

In JFileParser.java

public void run() {
     JFrame frame = new JFrame("Java File Parser");
     JFileParserController controller = new JFileParserController(frame);
     controller.setModel(new JFileParserModel());
     frame.setContentPane(new JFileParserPanel(controller));
     // etc.

In JFileParserController.java

public void actionPerformed(ActionEvent e) {
    if (e.getActionCommand().equals("Back") {
        model.prevPanel();
        frame.setTitle(model.getCurPanelName());
        // etc.

In JFileParserPanel.java

backButton.addActionListener(controller);
backButton.setActionCommand("Back");

nextButton.addActionListener(controller);
nextButton.setActionCommand("Next");

In JFileParserModel.java

private static final List PANEL_NAMES =
        Arrays.asList("New window", "File Chooser", "File parser");

-
Sorry if I was unclear earlier. The names of the subpanels should be in a constant list. Do not duplicate code by also making a separate string representation for each of them.

• My bad, usually people want a looping list. Storing the index should still work just check for whether the list is at the end before incrementing or decrementing.

Edit adding links for MVC:

Just found some good links that explain MVC more.

• This answer to GUI not working after rewriting to MVC

• Java SE Application Design With MVC

• This answer to MVC Progress Bar Threading

• This answer to Java MVC - How to divide a done text game into MVC?

StackExchange Code Review Q#13809, answer score: 3