7
\$\begingroup\$

I have recently undertaken a project for programming practice based on a simple text editor. Normally this kind of thing wouldn't bother me, but ever since I completed a software architectures module in college, I question everything about how my code is and should be formatted! (probably a good thing?)

I currently only have one class and while on the grand scale of things, it is not huge. I think it is maybe already too big considering the size of the project I am doing. Basically, I have a window with a text area and menu bar with a "File" and "Edit" menus on it with action listeners to perform functions when clicked.

To me the class just seems like it is very cluttered and has quite a lot of if-else if statements in the actionPerformed method. I was just wondering what yer opinion on the class length might be and if it can/should be broken into several other classes?

package com.schongeproductions.texteditor;

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Dimension;
import java.awt.Font;
import java.awt.Frame;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.print.PageFormat;
import java.awt.print.PrinterException;
import java.awt.print.PrinterJob;
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;

import javax.swing.BorderFactory;
import javax.swing.JFileChooser;
import javax.swing.JFrame;
import javax.swing.JMenu;
import javax.swing.JMenuBar;
import javax.swing.JMenuItem;
import javax.swing.JOptionPane;
import javax.swing.JScrollPane;
import javax.swing.JTextArea;
import javax.swing.border.Border;
import javax.swing.event.UndoableEditEvent;
import javax.swing.event.UndoableEditListener;
import javax.swing.undo.CannotUndoException;
import javax.swing.undo.UndoManager;

@SuppressWarnings("serial")
public class EditorGUI extends JFrame implements ActionListener {

    public static void main(String[] args) {
        new EditorGUI();
    }

    //============================================
    // FIELDS
    //============================================

    // Menus
    private JMenu fileMenu;
    private JMenu editMenu;
    private JMenuItem newFile, openFile, saveFile, saveAsFile, pageSetup, printFile, exit;
    private JMenuItem undoEdit, redoEdit, selectAll, copy, paste, cut;


    // Window
    private JFrame editorWindow;

    // Text Area
    private Border textBorder;
    private JScrollPane scroll;
    private JTextArea textArea;
    private Font textFont;

    // Window
    private JFrame window;

    // Printing
    private PrinterJob job;
    public PageFormat format;

    // Is File Saved/Opened
    private boolean opened = false;
    private boolean saved = false;

    // Record Open File for quick saving
    private File openedFile;

    // Undo manager for managing the storage of the undos
    // so that the can be redone if requested
    private UndoManager undo;

    //============================================
    // CONSTRUCTOR
    //============================================

    public EditorGUI() {
        super("JavaEdit");

        // Create Menus
        fileMenu();
        editMenu();

        // Create Text Area
        createTextArea();

        // Create Undo Manager for managing undo/redo commands
        undoMan();

        // Create Window
        createEditorWindow();
    }

    private JFrame createEditorWindow() {
        editorWindow = new JFrame("JavaEdit");
        editorWindow.setVisible(true);
        editorWindow.setExtendedState(Frame.MAXIMIZED_BOTH);
        editorWindow.setDefaultCloseOperation(EXIT_ON_CLOSE);
        // Create Menu Bar
        editorWindow.setJMenuBar(createMenuBar());
        editorWindow.add(scroll, BorderLayout.CENTER);
        editorWindow.pack();
        // Centers application on screen
        editorWindow.setLocationRelativeTo(null);

        return editorWindow;
    }

    private JTextArea createTextArea() {
        textBorder = BorderFactory.createBevelBorder(0, Color.RED, Color.RED);
        textArea = new JTextArea(30, 50);
        textArea.setEditable(true);
        textArea.setBorder(BorderFactory.createCompoundBorder(textBorder, BorderFactory.createEmptyBorder(2, 5, 0, 0)));

        textFont = new Font("Verdana", 0, 14);
        textArea.setFont(textFont);

        scroll = new JScrollPane(textArea, JScrollPane.VERTICAL_SCROLLBAR_ALWAYS, JScrollPane.HORIZONTAL_SCROLLBAR_ALWAYS);

        return textArea;        
    }

    private JMenuBar createMenuBar() {
        JMenuBar menuBar = new JMenuBar();
        setJMenuBar(menuBar);
        menuBar.add(fileMenu);
        menuBar.add(editMenu);

        return menuBar;
    }

    private UndoManager undoMan() {
        // Listener for undo and redo functions to document
        undo = new UndoManager();
        textArea.getDocument().addUndoableEditListener(new UndoableEditListener() {

            @Override
            public void undoableEditHappened(UndoableEditEvent e) {
                undo.addEdit(e.getEdit());
            }
        });

        return undo;
    }

    private void fileMenu() {
        // Create File Menu
        fileMenu = new JMenu("File");
        fileMenu.setPreferredSize(new Dimension(40, 20));

        // Add file menu items
        newFile = new JMenuItem("New");
        newFile.addActionListener(this);
        newFile.setPreferredSize(new Dimension(100, 20));
        newFile.setEnabled(true);

        openFile = new JMenuItem("Open...");
        openFile.addActionListener(this);
        openFile.setPreferredSize(new Dimension(100, 20));
        openFile.setEnabled(true);

        saveFile = new JMenuItem("Save");
        saveFile.addActionListener(this);
        saveFile.setPreferredSize(new Dimension(100, 20));
        saveFile.setEnabled(true);

        saveAsFile = new JMenuItem("Save As...");
        saveAsFile.addActionListener(this);
        saveAsFile.setPreferredSize(new Dimension(100, 20));
        saveAsFile.setEnabled(true);

        pageSetup = new JMenuItem("Page Setup...");
        pageSetup.addActionListener(this);
        pageSetup.setPreferredSize(new Dimension(100, 20));
        pageSetup.setEnabled(true);

        printFile = new JMenuItem("Print...");
        printFile.addActionListener(this);
        printFile.setPreferredSize(new Dimension(100, 20));
        printFile.setEnabled(true);

        exit = new JMenuItem("Exit");
        exit.addActionListener(this);
        exit.setPreferredSize(new Dimension(100, 20));
        exit.setEnabled(true);

        // Add items to menu
        fileMenu.add(newFile);
        fileMenu.add(openFile);
        fileMenu.add(saveFile);
        fileMenu.add(saveAsFile);
        fileMenu.add(pageSetup);
        fileMenu.add(printFile);
        fileMenu.add(exit);
    }

    private void editMenu() {
        editMenu = new JMenu("Edit");
        editMenu.setPreferredSize(new Dimension(40, 20));

        // Add file menu items
        undoEdit = new JMenuItem("Undo");
        undoEdit.addActionListener(this);
        undoEdit.setPreferredSize(new Dimension(100, 20));
        undoEdit.setEnabled(true);

        redoEdit = new JMenuItem("Redo");
        redoEdit.addActionListener(this);
        redoEdit.setPreferredSize(new Dimension(100, 20));
        redoEdit.setEnabled(true);

        selectAll = new JMenuItem("Select All");
        selectAll.addActionListener(this);
        selectAll.setPreferredSize(new Dimension(100, 20));
        selectAll.setEnabled(true);

        copy = new JMenuItem("Copy");
        copy.addActionListener(this);
        copy.setPreferredSize(new Dimension(100, 20));
        copy.setEnabled(true);

        paste = new JMenuItem("Paste");
        paste.addActionListener(this);
        paste.setPreferredSize(new Dimension(100, 20));
        paste.setEnabled(true);

        cut = new JMenuItem("Cut");
        cut.addActionListener(this);
        cut.setPreferredSize(new Dimension(100, 20));
        cut.setEnabled(true);

        // Add items to menu
        editMenu.add(undoEdit);
        editMenu.add(redoEdit);
        editMenu.add(selectAll);
        editMenu.add(copy);
        editMenu.add(paste);
        editMenu.add(cut);
    }

    // Method for saving files - Removes duplication of code
    private void saveFile(File filename) {
        try {
            BufferedWriter writer = new BufferedWriter(new FileWriter(filename));
            writer.write(textArea.getText());
            writer.close();
            saved = true;
            window.setTitle("JavaText - " + filename.getName());
        } catch (IOException err) {
            err.printStackTrace();
        }
    }

    // Method for quick saving files
    private void quickSave(File filename) {
        try {
            BufferedWriter writer = new BufferedWriter(new FileWriter(filename));
            writer.write(textArea.getText());
            writer.close();
        } catch (IOException err) {
            err.printStackTrace();
        }
    }

    // Method for opening files
    private void openingFiles(File filename) {
        try {
            openedFile = filename;
            FileReader reader = new FileReader(filename);
            textArea.read(reader, null);
            opened = true;
            window.setTitle("JavaEdit - " + filename.getName());
        } catch (IOException err) {
            err.printStackTrace();
        }
    }

    @Override
    public void actionPerformed(ActionEvent event) {
        if(event.getSource() == newFile) {
            new EditorGUI();
        } else if(event.getSource() == openFile) {
            JFileChooser open = new JFileChooser();
            open.showOpenDialog(null);
            File file = open.getSelectedFile();                
            openingFiles(file);
        } else if(event.getSource() == saveFile) {
            JFileChooser save = new JFileChooser();
            File filename = save.getSelectedFile();
            if(opened == false && saved == false) {
                save.showSaveDialog(null);
                int confirmationResult;
                if(filename.exists()) {
                    confirmationResult = JOptionPane.showConfirmDialog(saveFile, "Replace existing file?");
                    if(confirmationResult == JOptionPane.YES_OPTION) {
                        saveFile(filename);                        
                    }
                } else {
                    saveFile(filename);
                }
            } else {
                quickSave(openedFile);
            }
        } else if(event.getSource() == saveAsFile) {
            JFileChooser saveAs = new JFileChooser();
            saveAs.showSaveDialog(null);
            File filename = saveAs.getSelectedFile();
            int confirmationResult;
            if(filename.exists()) {
                confirmationResult = JOptionPane.showConfirmDialog(saveAsFile, "Replace existing file?");
                if(confirmationResult == JOptionPane.YES_OPTION) {
                    saveFile(filename);                        
                }
            } else {
                saveFile(filename);
            }
        } else if(event.getSource() == pageSetup) {
            job = PrinterJob.getPrinterJob();
            format = job.pageDialog(job.defaultPage());    
        } else if(event.getSource() == printFile) {
            job = PrinterJob.getPrinterJob();
            if(job.printDialog()) {
                try {
                    job.print();
                } catch (PrinterException err) {
                    err.printStackTrace();
                }
            }
        } else if(event.getSource() == exit) {
            System.exit(0);
        } else if(event.getSource() == undoEdit) {
            try {
                undo.undo();
            } catch(CannotUndoException cu) {
                cu.printStackTrace();
            }
        } else if(event.getSource() == redoEdit) {
            try {
                undo.redo();
            } catch(CannotUndoException cur) {
                cur.printStackTrace();
            }
        } else if(event.getSource() == selectAll) {
            textArea.selectAll();
        }  else if(event.getSource() == copy) {
            textArea.copy();
        } else if(event.getSource() == paste) {
            textArea.paste();
        } else if(event.getSource() == cut) {
            textArea.cut();
        }
    }

    //============================================
    // GETTERS AND SETTERS
    //============================================

    public JTextArea getTextArea() {
        return textArea;
    }

    public void setTextArea(JTextArea text) {
        textArea = text;
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ hey man, thanks for the code. May I use this code in my project? I'll assume the answer is yes until you explicitly say no. \$\endgroup\$
    – Monir
    Jan 22, 2015 at 17:16
  • \$\begingroup\$ I will post the improved/modified version once I'm done with it. \$\endgroup\$
    – Monir
    Jan 23, 2015 at 2:00
  • \$\begingroup\$ Sure...go nuts! \$\endgroup\$
    – Schonge
    Feb 4, 2015 at 6:48

3 Answers 3

11
\$\begingroup\$
  1. You do a editorWindow.setDefaultCloseOperation(EXIT_ON_CLOSE); which means if I open multiple windows using New menu item and close any one of the windows program exits. This is because you do not differentiate between a window and an application instance. you should separate your window code from your application instance your application instance should manage its open windows. A window should inform the application instance when it is closed and the application should exit when its last window is closed. If you do not exit at the first window-closed event, you should release memory, file handles, (network, db connectionsin the future?) etc whatever resources associated with the window.

  2. You get the file name before you show the file chooser you get a NullPointerException, which you do not catch and swing prints it to console, provided you run the program from console.

    JFileChooser save = new JFileChooser();
    File filename = save.getSelectedFile();
    if(opened == false && saved == false) {
        save.showSaveDialog(null);
    
  3. In your file I/O operation you do this:

    // Method for opening files
    private void openingFiles(File filename) {
        try {
            openedFile = filename;
            FileReader reader = new FileReader(filename);
            textArea.read(reader, null);
            opened = true;
            window.setTitle("JavaEdit - " + filename.getName());
        } catch (IOException err) {
            err.printStackTrace();
        }
    }
    

    Apart from possible resource leaks, as previously pointed out, you give the user no indication whether his command completed successfully or not. Probably when you try to save a file you would expect the editor to give some indication of the result: if successful a "file saved" notification in the status bar, the star indicating the file is modified to disappear etc. If unsuccessful you would want even more obvious indication, such as a message box, saying "File is open by another process" or at least "Could not save buffer".

  4. Your event handling code seems very AWT, with a single if/else if block. A better way is to use Actions instead. Check out the swing Action tutorial; it also has a demo, although not perfect, shows how a single action can be associated with a menu item, toolbar button, a keyboard shortcut etc.

\$\endgroup\$
7
\$\begingroup\$

The only thing that sticks out to me is in your save methods your not calling finally, and that is where your close call should be. This way if the IOException happens before close gets called your possibly holding a file handler open somewhere.

// Method for saving files - Removes duplication of code
private void saveFile(File filename) {
    try {
        BufferedWriter writer = new BufferedWriter(new FileWriter(filename));
        writer.write(textArea.getText());
        saved = true;
        window.setTitle("JavaText - " + filename.getName());
    } catch (IOException err) {
        err.printStackTrace();
    }
    finally
    {
          writer.close();
    }
}

// Method for quick saving files
private void quickSave(File filename) {
    try {
        BufferedWriter writer = new BufferedWriter(new FileWriter(filename));
        writer.write(textArea.getText());
    } catch (IOException err) {
        err.printStackTrace();
    }
    finally
    {
         writer.close();
    }
}

See "The finally Block"

Do the same for your FileReaders as well.

\$\endgroup\$
0
\$\begingroup\$

Code like this should raise a flag:

    // Add file menu items
    undoEdit = new JMenuItem("Undo");
    undoEdit.addActionListener(this);
    undoEdit.setPreferredSize(new Dimension(100, 20));
    undoEdit.setEnabled(true);

    redoEdit = new JMenuItem("Redo");
    redoEdit.addActionListener(this);
    redoEdit.setPreferredSize(new Dimension(100, 20));
    redoEdit.setEnabled(true);

If you can squint your eyes and two paragraphs look the same, you are not DRY.

I usually look at these and try to figure out what is similar/different. In your case, with a single exception, the only difference is the menu name and the action that menu name takes. So that entire block can become

"Undo", "Redo", "Select All", "Copy", "Paste", "Cut".

for the most part. However, you must still "bind" the word "Undo" with some code.

So one way to do this might be with a Lambda:

addMenuItem("Undo", {->undo.undo()})
addMenuItem("Redo", {->undo.redo()})
...

the addMenuItem is a method you would define that adds the method name and lambda to a data structure for your actionPerformed method to use. It will probably need to create a map of <JMenuItem, Runnable> so that your actionPerformed can just:

eventMap.get(event.getSource()).call();

If you do it right, this can be your ENTIRE actionPerformed method (Well, you probably want a try/catch around this line to catch/report failures).

There are other complexities that this EXACT solution doesn't solve--nesting comes to mind (sub-menus) but those can be overcome by adding additional parameters to the addMenuItem function (such as "Parent" to specify a parent menu to place this menu under).

This kind of coding/refactor is important for a few reasons:

  1. copy/paste code tends to be very difficult to debug, the differences meld into the pattern.
  2. The supporting code is not difficult to write once you get in the habit and will probably save you more time than it takes.
  3. It's more fun to write code where you have to think than copy/paste/debug
  4. Once in this form it is often possible to both find higher level refactors and/or externalize the data (put it in a config file).

One trick here--don't FORCE every instance to use the new addMenuItem method. By just not converting exception cases (In your case, consider "Edit" an exception case) you can keep addMenuItem quite simple. The hardest thing about coding is exception cases, the less you have the simpler your code can be.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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