5
\$\begingroup\$

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;

    private static final String
        EMPTY_PANEL = "New window",
        FILE_CHOOSER = "File chooser",
        FILE_PARSER = "File parser"
    ;
    private List< String > deck = Arrays.asList(
        EMPTY_PANEL,
        FILE_CHOOSER,
        FILE_PARSER
    );

    public Deck() {
        setLayout( new CardLayout() );
        layout = ( CardLayout ) getLayout();
    }

    public void setNavigator( DeckNavigator navigator ) {
        this.navigator = navigator;
    }

    public void initDeck() {
        add( new JPanel(), EMPTY_PANEL );
        add( new ChooseFile(), FILE_CHOOSER );
        add( new ParseFile(), FILE_PARSER );

        navigator.setView( EMPTY_PANEL );
    }

    public void nextView( String view ) {
        int currentView = deck.indexOf( view );
        if( currentView != deck.size() - 1 ) {
            setView( currentView + 1 );
        }
    }

    public void previousView( String view ) {
        int currentView = deck.indexOf( view );
        if( currentView > 0 ) {
            setView( currentView - 1 );
        }
    }

    private void setView( int card ) {
        String view = deck.get( card );
        layout.show( this, view );
        navigator.setView( view );
    }
}

DeckNavigator.java

package my.controllers;

import my.views.Deck;

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.JButton;
import javax.swing.JPanel;

public class DeckNavigator extends JPanel {

    private Deck deck;
    private String view;

    public DeckNavigator() {
        addPrevious();
        addNext();
    }

    public void setDeck( Deck deck ) {
        this.deck = deck;
    }

    public void setView( String view ) {
        this.view = view;
    }

    private boolean deckIsSet() {
        return deck != null;
    }

    private boolean viewIsSet() {
        return view != null;
    }

    private void addPrevious() {
        JButton button = new JButton( "Back" );
        button.addActionListener( new ActionListener() {

            @Override
            public void actionPerformed( ActionEvent e ) {
                if( deckIsSet() && viewIsSet() ) {
                    deck.previousView( view );
                }
            }
        } );

        add( button );
    }

    private void addNext() {
        JButton button = new JButton( "Next" );
        button.addActionListener( new ActionListener() {

            @Override
            public void actionPerformed( ActionEvent e ) {
                if( deckIsSet() && viewIsSet() ) {
                    deck.nextView( view );
                }
            }
        } );

        add( button );
    }
}

Notes:

  • My packages aren't that vague, I just abstracted them before posting this.
  • The logger is a pseudo class. All it does is print to the console for now. Though I'm not using it as much now...
  • My first panel is blank to simulate the need to click on the appropriate menu item

Questions

  1. When using the invokeLater() method in my main class, would it be better, or possible to use this instead of getting a new instance of the same class? I just thought of this, so I haven't even tried it or looked it up yet. Edit: Just tested this, appears it doesn't work because this is not static, worth a shot.
  2. After many hours on google, and very little sleep, the only way I could think to get my panels to communicate with each other was to do as you've just seen. And I didn't come up with that solution until after I did get some sleep. I say only because I don't think writing to a temp file is a very elegant solution in this instance. Is there a better way to do this? Already I'm seeing issues when I want another class to be able to change the view, and I'd hate to have to pass the deck and navigator to every panel on the off chance they or their children will need it at some point. I know I can streamline it by extending a parent class with those methods, but just curious what y'all have to say.
  3. Is there a better way to navigate and/or store the deck? I initially thought of combining the "titles" and panels into an associative array, or HashMap as I've come to know they're called in Java, that way I could iterate over it to initialize the deck instead of having to explicitly call each panel. But that immediately presented problems when I tried to navigate sequentially through them as I couldn't get their position with indexOf(). I thought of creating an iterator for it, but, I admit, I'm not very familiar with the concept and didn't see a "previous" method available in the interface, which makes me think I would have to create a completely different iterator and deck but backwards if I wanted to step down.

Please feel free to elaborate on other points than just those I listed. I'm still trying to learn, so any help is appreciated.

\$\endgroup\$
3
\$\begingroup\$

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<String> 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.

\$\endgroup\$
  • \$\begingroup\$ Thank you for your answer, I'm particularly grateful for the links and will be looking over them shortly. I had figured that out about this, thus the edit. Thank you none-the-less I wasn't sure exactly why, only that it was because it wasn't static. \$\endgroup\$ – mseancole Jul 23 '12 at 16:17
  • \$\begingroup\$ You are quite right about DeckNavigator, not sure what I was thinking. This is only a simple program at this stage. I am adding on to it in small easily manageable phases, thus the need for the MVC pattern. I noticed that second link has much on the observer pattern. This is something I had found over the weekend and was looking into. It looks very much like what I am already doing, but slightly easier. I was already thinking of implementing it, but thank you for the link, I'm curious about what other methods there are. \$\endgroup\$ – mseancole Jul 23 '12 at 16:17
  • \$\begingroup\$ Eventually these cards are going to be a part of a popup from a main window. I was thinking about how to use those subpanel names to change the JFrame title. In this instance should they still not be constant? Why should they not be constant now? I was under the impression that any variables you did not want or expect to change should be constant. \$\endgroup\$ – mseancole Jul 23 '12 at 16:17
  • \$\begingroup\$ I was originally trying to use a list, but I needed a key/value pair and I could not figure out how to do so with a list. List< String, JPanel > doesn't seem to work, as it says that List only expects a single element. Also, unless I am mistaken, it appears that your example loops back around once it has reached the min/max. I want linear navigation, not circular. \$\endgroup\$ – mseancole Jul 23 '12 at 16:17
  • \$\begingroup\$ @showerhead Why do you need to keep a reference to the subpanels? It seems like you just need the list of names to change to the right panel. What else are you doing with the subpanels? \$\endgroup\$ – Eva Jul 23 '12 at 20:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.