HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

First Project: JFrame Class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
projectfirstclassjframe

Problem

This is my second "first" attempt at starting Java. My first was cut short when I had to go back to a PHP project that required my immediate attention. But perhaps that was a good thing. I learned quite a bit more about OOP, better coding practices, and some other topics. So, after this hiatus, I noticed quite a bit wrong with my first attempt and ended up scraping it and starting from scratch.

So here is the first phase of my Java Project: The Second "First" Attempt. Sounds like a horrible sequel to a horrible movie. The project, once complete, will convert a text file to a CSV file. The format is custom, so it is unlikely that preexisting libraries will help me here, so I didn't even bother looking, except for guidelines on how to go about it. This first class is relatively simple and creates a window with some instructions on how to use the program and a button that allows the user to begin the process by loading a text file. Not very exciting yet, but that's why its only the first phase. Please take a look at it and let me know how you think this can be improved.

I have some specific questions and concerns, located below the code, however there are a number of them and they are quite lengthy. So if you just wish to review the code and skip the questions, that's fine too.

```
package view;

import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JLabel;
import javax.swing.JButton;

import java.awt.FlowLayout;

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

import javax.swing.JFileChooser;
import javax.swing.filechooser.FileFilter;
import javax.swing.filechooser.FileNameExtensionFilter;

public class JavaFileParser extends JFrame implements ActionListener {
/*------------------------------------------------------------------------------
CONSTRUCT
------------------------------------------------------------------------------*/
public static void main( String[] args ) {
JavaFileParser parser = new JavaFi

Solution

Addressing Questions and Concerns

  • What you have so far is typical for a Java view. The ActionListener (or any listener for that matter) should be moved to the Controller.



  • Your imports ain't nothing. I'm not familiar with PHP, but in Java, every class is its own file (for the most part), so imports get bulky fast. Good job on not using wildcard imports (like javax.swing.*) that pollute the namespace.



  • When people say you should only extend JFrame when your class IS a JFrame, they mean you will be fiddling with stuff that only apply to a frame top-level container, such as modding the title bar. The additions you have could easily go into a JApplet. I would switch to subclassing JPanel. BTW, if you plan on doing custom drawing (not applicable here but maybe in the future), it's customary to subclass JComponent rather than JPanel. (see Subclass JComponent or JPanel) As for the ActionListener, I would put it in the Controller as I mentioned before. UI goes with the Controller, not the View.



  • I would separate the View from the main program. Naming varies for the class with the main method. Some people favor Main, some like suffixing with Program, it's really up to you. (see this discussion on possible names) However, I would avoid naming it index because it's lowercased, and because it's typically used for web-related stuff.



  • I tend to see people use less of the this keyword in Java. Sun's tutorial only mentions two uses: disambiguating a field and calling a constructor from another constructor. (see also When Should I Use "This" In a Class) Personally, I don't see any problem with using this to make instance variables stand out. But I've also seen people use prefixing (e.g. mButton) to distinguish them. The this keyword before methods is kinda strange.



  • For this one I'm not sure. This question kinda gets at what you're asking sorta. It's probably safer to remove them just in case. Sorry.



  • I think when you say lambda function you mean anonymous inner class. Java doesn't have lambdas. I've seen some dang long anonymous listeners in my time that made sense in context, but in your case, I'd go with that Controller class I keep mentioning.



Additional Notes

  • Most people just prefix with J rather than Java, e.g. JFrame or JTetris.



-
Everything related to swing should be in the event-dispatching thread. If you put your call to parser.setVisible(true) in a call to SwingUtilites.invokeLater(Runnable), it will execute your code in the event-dispatching thread. (see Threads and Swing) This is where either an anonymous inner class or a separate main class will come in handy. You can either do this

public class JFileParserProgram implements Runnable {

     @Override
     public void run() {
         JFrame frame = new JFrame("Title");
         frame.setContentPane(new JFileParserView());
         frame.pack();
         frame.setLocationRelativeTo(null);
         frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
         frame.setVisible(true);
     }

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

}


or this

public class JFileParserProgram {

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {

            @Override
            public void run() {
                JFrame frame = new JFrame("Title");
                frame.setContentPane(new JFileParserView());
                frame.pack();
                frame.setLocationRelativeTo(null);
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.setVisible(true);
            }

        }
    }

}


I personally use the first way because when I think of things to run, I think programs.

Code Snippets

public class JFileParserProgram implements Runnable {

     @Override
     public void run() {
         JFrame frame = new JFrame("Title");
         frame.setContentPane(new JFileParserView());
         frame.pack();
         frame.setLocationRelativeTo(null);
         frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
         frame.setVisible(true);
     }

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

}
public class JFileParserProgram {

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {

            @Override
            public void run() {
                JFrame frame = new JFrame("Title");
                frame.setContentPane(new JFileParserView());
                frame.pack();
                frame.setLocationRelativeTo(null);
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.setVisible(true);
            }

        }
    }

}

Context

StackExchange Code Review Q#13482, answer score: 7

Revisions (0)

No revisions yet.