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

Simple text editor class

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

Problem

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)

Solution

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

-
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);


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

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

Code Snippets

JFileChooser save = new JFileChooser();
File filename = save.getSelectedFile();
if(opened == false && saved == false) {
    save.showSaveDialog(null);
// 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();
    }
}

Context

StackExchange Code Review Q#51175, answer score: 11

Revisions (0)

No revisions yet.