9
\$\begingroup\$

Please give feedback on all aspects of the code, whether it be the commenting, usage of methods, semantics or anything that can be improved.

This code looks flawless to me, so any suggestions for improvements will make a difference.

//  Imports packages for AWT, Events, Scanner, File, PrintWriter,
//  exceptions and Swing components.
import java.util.Scanner;
import java.io.*;
import java.awt.*;
import java.awt.event.*;
import javax.swing.*;

/**
 * This class extends JFrame to inherit all methods for active use.
 * This class implements the interface ActionListener to handle user actions
 */
public class SchultzTobias extends JFrame implements ActionListener {

    //  Declaring all swing components for access throughout multiple methods
    private         JMenuBar      menuBar;
    private         JTextArea     txtArea;
    private final   JMenu[]       menu = new JMenu[4];
    private final   JMenuItem[]   menuItem = new JMenuItem[7];
    private         JMenu         fontMenu;
    private final   JMenuItem[]   fontItem = new JMenuItem[10];

    /**
     * This constructor initializes all components, sets the workspace
     * preferences and connects everything to the JFrame superclass
     */
    public SchultzTobias() {

        //  Assigns a title to the notepad
        super("Simple Notepad");

        //  Initializes the text area for user input
        txtArea = new JTextArea(24, 54);

        //  Sets formatting to break line at whole words, and adding
        //  some border to the JTextArea.
        txtArea.setLineWrap(true);
        txtArea.setWrapStyleWord(true);
        txtArea.setBorder(BorderFactory.createEmptyBorder(10, 10, 10, 10));

        // If content exceeds size of viewport, make scrolling through the
        // content possible
        JScrollPane scrollPane = new JScrollPane(txtArea);

        //  initializes all menus, menuItems and corresponding key bindings
        initializeMenus();
        initializeKeyBindings();

        //  adding menus and JTextArea to the JFrame
        add(menuBar);
        add(scrollPane, BorderLayout.SOUTH);

        //  Presenting the JFrame, and stating the programs close operation
        setVisible(true);
        pack();
        setDefaultCloseOperation(EXIT_ON_CLOSE);
    }

    /**
     * This method constructs and adds names and actionListener's to
     * JMenu's and JMenuItem's, which are then added to the JMenuBar.
     */
    public void initializeMenus() {

        //  Declaring the menu bar
        menuBar = new JMenuBar();

        //  Assigns names to the main menu and adding them to
        //  actionListener's and the menu bar
        String[] menuNames = {"File", "Edit", "Format", "Help"};

        for (int i = 0; i < menu.length; i++) {
            menu[i] = new JMenu(menuNames[i]);
            menu[i].addActionListener(this);
            menuBar.add(menu[i]);
        }

        //  Assigns names to the menu items and adding them to
        //  actionListener's and the main menu
        String[] itemString = {"Open", "Save", "Exit", "Text Wrap",
                "No Text Wrap", "Clear", "About Simple Notepad"};

        for (int i = 0; i < menuItem.length; i++) {
            menuItem[i] = new JMenuItem(itemString[i]);
            menuItem[i].addActionListener(this);

            if (i < 3)      { menu[0].add(menuItem[i]);} // Adding to 'File'
            else if (i < 5) { menu[1].add(menuItem[i]);} // Adding to 'Edit'
            else if (i < 6) { menu[2].add(menuItem[i]);} // Adding to 'Format'
            else            { menu[3].add(menuItem[i]);} // Adding to 'Help'
        }

        // Initializing the menu for fonts and adding
        // menu items with font sizes
        fontMenu = new JMenu("Font");

        for (int i = 0; i < fontItem.length; i++) {
            fontItem[i] = new JMenuItem(String.valueOf((i + 1) * 2));
            fontItem[i].addActionListener(this);
            fontMenu.add(fontItem[i]);
        }

        //  Adding the 'Font' menu to the 'Edit' menu
        menu[1].add(fontMenu);
    }

    /**
     * This method assigns a mnemonic character for each menu and menuItem.
     * If the program is viewed on a Mac, alt(option) key must be pressed
     * to view the mnemonic icon.
     */
    public void initializeKeyBindings() {

        //  The assignment had mnemonic icons on different indexes in its
        //  presentation. To deliver exactly what the assignment specified,
        //  and to keep the code readable, i wrote each mnemonic statement
        //  separately.
        menu[0].setMnemonic(KeyEvent.VK_F);     //  File
        menu[1].setMnemonic(KeyEvent.VK_E);     //  Edit
        menu[2].setMnemonic(KeyEvent.VK_O);     //  Format
        menu[3].setMnemonic(KeyEvent.VK_H);     //  Help

        menuItem[0].setMnemonic(KeyEvent.VK_O); //  Open
        menuItem[1].setMnemonic(KeyEvent.VK_S); //  Save
        menuItem[2].setMnemonic(KeyEvent.VK_E); //  Exit
        menuItem[3].setMnemonic(KeyEvent.VK_W); //  Text Wrap
        menuItem[4].setMnemonic(KeyEvent.VK_N); //  No Text Wrap
        menuItem[5].setMnemonic(KeyEvent.VK_C); //  Clear
        menuItem[6].setMnemonic(KeyEvent.VK_A); //  About

        fontMenu.setMnemonic(KeyEvent.VK_F);    //  Font
    }

    /**
     * This method prompts user to enter name of file that will be attempted
     * to be opened
     *
     * @exception FileNotFoundException On input error.
     */
    public void openFile() {

        //  Prompts user for the name of the file to read
        String fileOutput = "";
        String fileName = JOptionPane.showInputDialog(txtArea, "Oppgi filnavn");

        //  If the user has typed anything, attempt to open file
        if (fileName != null) {
            try {

                //  Read everything from file until the end-anchor is found: \Z
                Scanner content = new Scanner(new File(fileName));
                content.useDelimiter("\\Z");

                while (content.hasNext()) {
                    fileOutput += content.next();
                }

                //  Store content read from file in text area and close dialog
                //  box.
                txtArea.setText(fileOutput);
                content.close();

                //  If file was not found, print the exception that was thrown
            } catch (FileNotFoundException fnfe) {
                System.out.println("This error was thrown: " + fnfe);
            }
        }
    }

    /**
     * This method  prompts user to enter name of file that will be attempted
     * to be stored
     *
     * @exception IOException On output error.
     */
    public void saveFile() {

        //  Prompts user for the name of the file to be stored
        String fileName = JOptionPane.showInputDialog(txtArea, "Oppgi filnavn");

        //  If user has typed in anything, attempt to save file
        if (fileName != null) {

            //  Copy content from the text area into the file being created
            try {
                PrintWriter content = new PrintWriter(new File(fileName));
                content.write(txtArea.getText());
                content.close();

                //  If there was an error, print the exception
            } catch (IOException ioe) {
                System.out.println("This error was thrown: " + ioe);
            }
        }
    }

    /**
     *  This method prompts user with three options
     *      - Save text area to file through method saveFile()
     *      - Exit the program without storing file
     *      - Go back to superclass JFrame
     *
     * @param clear handles what to do after a user has selected an option.
     *              If clear is true, do not exit program after statement.
     *              If clear is false, exit program after statement.
     */
    public void exitFile(boolean clear) {

        //  Prompts user with options to store file, not to store file or
        int options = JOptionPane.showConfirmDialog(
                txtArea, "Vil du lagre først?");

        //  If user acted on the YES option
        if (options == JOptionPane.YES_OPTION) {
            saveFile();

            // And first acted on the 'Clear' menuItem
            if (clear) {
                txtArea.setText("");

            // And first acted on the 'Exit' menuItem
            } else {
                System.exit(0);
            }
        }

        //  If user acted on the NO option
        else if (options == JOptionPane.NO_OPTION) {

            // And first acted on the 'Clear' menuItem
            if (clear) {
                txtArea.setText("");

            // And first acted on the 'Exit' menuItem
            } else {
                System.exit(0);
            }
        }
    }

    /**
     * This method handles user actions on the menu's
     *
     * @param e handles actionListeners
     */
    @Override
    public void actionPerformed(ActionEvent e) {

        //  Stores a variable with the source of the action event for
        //  readable event handling
        Object source = e.getSource();

        //  Names of menuItems that the user can act on is a comment to its
        //  preceding code
        //  Open:
        if (source == menuItem[0]) {
            openFile();
        }

        //  Save:
        if (source == menuItem[1]) {
            saveFile();
        }

        //  Exit:
        if (source == menuItem[2]) {

            //  Exit's the file directly if JTextArea is empty
            if (txtArea.getText().equals("")){
                System.exit(0);
            }
            else {
                exitFile(false);
            }
        }

        //  Text Wrap:
        if (source == menuItem[3]) {
            txtArea.setLineWrap(true);
            txtArea.setWrapStyleWord(true);
        }

        //  No Text Wrap:
        if (source == menuItem[4]) {
            txtArea.setLineWrap(false);
            txtArea.setWrapStyleWord(false);
        }

        //  Font:
        //  Assigns new font size to match the users size preference entered
        for (int i = 0; i < fontItem.length; i++) {
            if (source == fontItem[i]) {
                txtArea.setFont(new Font("Serif", Font.PLAIN,
                        ((i + 1) * 2)));
            }
        }

        //  Clear:
        //  If there is content in JTextArea, prompt user to save content
        if (source == menuItem[5] && !txtArea.getText().equals("")) {
                exitFile(true);
        }

        //  About:
        if (source == menuItem[6]) {
            JOptionPane.showMessageDialog(txtArea,
                    "Denne applikasjonen er laget av\nTobias Goulden Schultz" +
                            "\n\nDet er en enkel tekstbehandler.");
        }
    }

    /**
     * This method is main, it declares and initializes a new Object of
     * the class SchultzTobias
     */
    public static void main(String[] args) {
        SchultzTobias st = new SchultzTobias();
    }
}
\$\endgroup\$
  • \$\begingroup\$ I'm not sure is it intended, but your option for changing font size, changes also the font itself(from Dialog to Serif). \$\endgroup\$ – m.cekiera May 28 '15 at 8:50
3
\$\begingroup\$

Automatic Resource Management

Scanner is Closable, so you can use the automatic resources:

try (Scanner content = new Scanner(new File(fileName))) {
    ... rest of code goes here, don't need a content.close();
} catch (FileNotFoundException fnfe) {
    System.out.println("This error was thrown: " + fnfe);
}

Menus

There are lots of options for implementing menus. Using arrays and referring to indexes to determine which menu they go under and to find out which menu was chosen works okay for small applications, but doesn't scale and requires multiple places to change index references when you decide to reorganize the menus.

If you use subclasses of javax.swing.AbstractAction, you can keep the logic for the actions separate. Also, later on, you could add the same action to both a menu item and a toolbar. See the Action javadoc for properties you can put into an action. You can create a subclass of AbstractAction with multiple constructors and helper methods for setting and getting description, icon, mnemonic, etc.


Comments

You don't need to begin JavaDoc comments with "This class ..." or "This method ...". See the Method descriptions begin with a verb phrase section.

Some boilerplate does not need comments. For example, the "imports" section does not really need a comment. Also, some comments describe a little too much what is going on, and not the why (e.g. instead of "Sets formatting to break line at whole words ...", you could just have one comment "Create and format the JTextArea", unless there's a line you think will be particularly likely to forget why it's there.

Not a big issue, but plurals in English don't use apostrophe (e.g. menu's can be menus).


EDITED TO ADD:

exitFile() could be shortened while keeping the same functionality. Your options are only YES_OPTION and NO_OPTION, and there is duplicate code between them that can just execute regardless:

public void exitFile(boolean clear) {

    //  Prompts user with options to store file, not to store file or
    int options = JOptionPane.showConfirmDialog(
            txtArea, "Vil du lagre først?");

    //  If user acted on the YES option
    if (options == JOptionPane.YES_OPTION) {
        saveFile();
    }

    // And first acted on the 'Clear' menuItem
    if (clear) {
        txtArea.setText("");

    // And first acted on the 'Exit' menuItem
    } else {
        System.exit(0);
    }
}
\$\endgroup\$
4
\$\begingroup\$

give feedback on all aspects of the code

so, here is a short feedback:

few things i've noticed,

  • almost all methods are public (is this needed?)
  • e.g. initializeMenus() is called from within ctor, but is is not final
  • class SchultzTobias can be subclassed (it's not final)
  • most of your methods can be overridden (when subclassed...)
  • what is menuItem[3] and menu[2] ? (you only have few static menu-items, maybe you should consider using mnuItmSave,mnuItmOpen,... for better reading)
  • exitFile(boolean clear) can be shortend (look at it, you have duplicate code inside!)
  • exitFile(boolean clear) is only called by MenuItem-Action, not when closing the window (e.g. with windows X-Button)
  • calling setVisible() inside of ctor works, but it's unusual (and maybe unexpected when subclassed)
  • you don't close Closeables in case of exception (like: PrintWriter)

Cheers!

\$\endgroup\$

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.