patternjavaMinor
Beginner text editor
Viewed 0 times
beginnertexteditor
Problem
I wrote a simple text editor and I would like to get some critical comments. Below I present only a main part of code. I removed icons, irrelevant functions, etc. I am aware,that JTextArea is not best choice for text editor, however I purposely didn't choose JTextPane because at my level it seems too complicated.
I would be very grateful for a comments on:
Any other advice will be appreciated.
`import javax.swing.*;
import javax.swing.text.*;
import java.awt.*;
import java.awt.event.*;
import java.io.*;
import java.lang.Object;
public class SimpleTextEditor{
private JFrame frame;
private Action newAction,openAction,saveAction,saveAsAction,boldAction,italicsAction,copyAction,cutAction,pasteAction,printAction,findAction;
private JTextArea mainTextArea;
private JTextField findField,replaceField;
private JFileChooser fileChooser;
private File file;
private int boldCheck,italicsCheck,caseCheck,replaceCheck,keepSearch;
private JComboBoxfontTypeList,fontSizeList;
private JButton findNext,replace,findAll,replaceAll,okFind,find,previousFind;
private JDialog findDialog;
private JCheckBox caseSensitive;
private Highlighter highlighter;
public static void main(String[]args){
SimpleTextEditor simpleNote=new SimpleTextEditor();
simpleNote.go();
}
private void go(){
frame=new JFrame();
JPanel background = new JPanel();
frame.
I would be very grateful for a comments on:
- code structure - is it a good idea, to put everything in the one class, or it would be better to divide it between couple of classes?
- division of tasks between classes - is it good solution to use multiple methods to create different parts of a GUI?
- events handling - is it good practice to cumulate Action Listeners of multiple objects under one class? or is it better and more transparent to create one class per action or per group of related actions?
- fields - is there too many of them? what is a good alternative for them? getter method?
Any other advice will be appreciated.
`import javax.swing.*;
import javax.swing.text.*;
import java.awt.*;
import java.awt.event.*;
import java.io.*;
import java.lang.Object;
public class SimpleTextEditor{
private JFrame frame;
private Action newAction,openAction,saveAction,saveAsAction,boldAction,italicsAction,copyAction,cutAction,pasteAction,printAction,findAction;
private JTextArea mainTextArea;
private JTextField findField,replaceField;
private JFileChooser fileChooser;
private File file;
private int boldCheck,italicsCheck,caseCheck,replaceCheck,keepSearch;
private JComboBoxfontTypeList,fontSizeList;
private JButton findNext,replace,findAll,replaceAll,okFind,find,previousFind;
private JDialog findDialog;
private JCheckBox caseSensitive;
private Highlighter highlighter;
public static void main(String[]args){
SimpleTextEditor simpleNote=new SimpleTextEditor();
simpleNote.go();
}
private void go(){
frame=new JFrame();
JPanel background = new JPanel();
frame.
Solution
You are asking all the right questions :)
code structure - is it a good idea, to put everything in the one class, or it would be better to divide it between couple of classes?
No, putting everything in one giant class is not a good idea at all. It makes it impossible to reuse part of the code in other projects, it makes it really hard to find the code you are interested in, and it will also make it very hard to add new functionality later on (because you can't just concentrate on the relevant classes; you only have one class, so everything is relevant all the time).
A class should ideally only be responsible for one thing. So you might have a model class, which holds the entered text, and offers methods to manipulate it (such as search something in it, replace, etc). Saving/Loading should also happen in a separate class. And you could separate some gui elements into their own class as well (such as main menu, toolbar, search-and-replace window, etc).
You should also definitely split up your actions class. Give each action it's own class. And never use strings like this for program flow (if you must use your approach, use enums).
division of tasks between classes - is it good solution to use multiple methods to create different parts of a GUI?
Yes, definitely. Methods should - just like classes - really only be responsible for one thing. Ideally, this makes them reusable, and it definitely makes your code easier to read.
events handling - is it good practice to cumulate Action Listeners of multiple objects under one class? or is it better and more transparent to create one class per action or per group of related actions
No, such an accumulation is not very good, as it's really hard to read and maintain.
fields - is there too many of them? what is a good alternative for them? getter method?
Yes, you have way too many fields, which makes your code quite hard to read. If you split your code up into multiple classes, you should already see a reduction in fields. Then, you might also notice that some fields don't actually need to be fields, but could be declared at method levels.
Misc
code structure - is it a good idea, to put everything in the one class, or it would be better to divide it between couple of classes?
No, putting everything in one giant class is not a good idea at all. It makes it impossible to reuse part of the code in other projects, it makes it really hard to find the code you are interested in, and it will also make it very hard to add new functionality later on (because you can't just concentrate on the relevant classes; you only have one class, so everything is relevant all the time).
A class should ideally only be responsible for one thing. So you might have a model class, which holds the entered text, and offers methods to manipulate it (such as search something in it, replace, etc). Saving/Loading should also happen in a separate class. And you could separate some gui elements into their own class as well (such as main menu, toolbar, search-and-replace window, etc).
You should also definitely split up your actions class. Give each action it's own class. And never use strings like this for program flow (if you must use your approach, use enums).
division of tasks between classes - is it good solution to use multiple methods to create different parts of a GUI?
Yes, definitely. Methods should - just like classes - really only be responsible for one thing. Ideally, this makes them reusable, and it definitely makes your code easier to read.
events handling - is it good practice to cumulate Action Listeners of multiple objects under one class? or is it better and more transparent to create one class per action or per group of related actions
No, such an accumulation is not very good, as it's really hard to read and maintain.
fields - is there too many of them? what is a good alternative for them? getter method?
Yes, you have way too many fields, which makes your code quite hard to read. If you split your code up into multiple classes, you should already see a reduction in fields. Then, you might also notice that some fields don't actually need to be fields, but could be declared at method levels.
Misc
- use more spaces to increase readability (eg around
=,==, etc).
- don't declare multiple fields in one line, it makes it easy to overlook a field (and it makes it easy to not realize that way too many fields were added :) ).
- don't just print the stacktrace of exceptions, but handle them (maybe your users write something important). For example, if you couldn't save, display a popup box notifying the user of this (and give them the opportunity to copy their work).
Context
StackExchange Code Review Q#84887, answer score: 8
Revisions (0)
No revisions yet.