patternjavaModerate
Simple text editor class
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
```
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)
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
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
to console, provided you run the program from console.
-
In your file I/O operation you do this:
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
to use
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.
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 itto 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.