Re: I develop a Java program to format Java codes
On 03/02/2012 03:29 AM, gearss8888@gmail.com wrote:
The program can format Java codes, the codes formatted will have correct indent spaces.
//----------
/*
Modified: 2012-3-1
it can format java codes so as to get the correct indent spaces.
*/
import java.io.IOException;//?????????
import java.io.*;
It is better to use single-type imports than import-on-demand. Certainly don't
do both on the same types.
import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.io.InputStreamReader;
import java.io.StringReader;
import java.io.StringWriter;
import java.io.FileInputStream;
import java.io.FileReader;
import java.io.InputStream;
import java.io.Reader;
import java.util.ArrayList;
import javax.swing.JOptionPane;
import java.awt.event.*;
import java.awt.*;
import javax.swing.*;
import java.awt.Graphics;
Don't mix single-type imports and import-on-demand for the same types.
import java.awt.Color;
import javax.swing.JFileChooser;
import javax.swing.SwingConstants;
import javax.swing.UIManager;
public class Tidy extends JFrame {
JButton SelectFilebtn, OKbtn;
The Java naming convention is to use lower-case first letter in variable and
method identifiers, and camel case thereafter. Thus, 'selectFileBtn', not
'SelectFilebtn'.
JTextField FileNameField;
String TabBlank = " "; // ?????????tab????????????????????????????????????????????????
Is there a tab in there? If so, you should use '\t' rather than an embedded TAB.
int HowManyTab = 0;
There is no need to set the default value twice like this, except there's
really no harm, either. In this case you could argue that the redundant
initialization helps document your intent.
boolean HaveUnCleanRow = false; // ????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????
boolean ForWhileIfBlock = false; // ?????????????????????for, while or if ??????????????????{}??????????????????????????????????????????
public Tidy () {
Your constructor is a bit unwieldy - too long and complicated. I suggest that
you refactor the fancy logic into a separate private method.
this.setTitle( "???????????????");
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
this.setLayout(new GridBagLayout());
GridBagConstraints gbc = new GridBagConstraints();
gbc.anchor = GridBagConstraints.WEST; //??????Layout?????????
gbc.insets = new Insets(2,2,2,2); //????????????????????????(???,???,???,???)
SelectFilebtn = new JButton("????????????");
SelectFilebtn.addActionListener(new ActionListener() {
public void actionPerformed(ActionEvent e) {
do_SelectFilebtn_actionPerformed(e);
}
});
FileNameField = new JTextField("",40);
gbc.gridy=1;
gbc.gridx=0;
this.add(SelectFilebtn, gbc);
gbc.gridx=1;
this.add(FileNameField, gbc);
//--------------
OKbtn = new JButton("OK");
OKbtn.addActionListener(new ActionListener() {
public void actionPerformed(ActionEvent e) {
String FileName = FileNameField.getText();
do_OKbtn_actionPerformed(e, FileName);
}
});
gbc.gridy=2;
gbc.gridx=0;
this.add(OKbtn, gbc);
//---------------------
this.pack();
this.setVisible(true);
}
I strongly recommend that the 'setVisible()' call happen after the constructor.
protected void do_SelectFilebtn_actionPerformed (ActionEvent e) {
Why is this method 'protected'?
JFileChooser chooser = new JFileChooser();// ?????????????????????
int option = chooser.showOpenDialog(this);// ???????????????????????????
if (option == JFileChooser.APPROVE_OPTION) {// ??????????????????????????????
File file = chooser.getSelectedFile();// ????????????????????????
FileNameField.setText(file.getAbsolutePath());// ?????????????????????????????????????????????
}
}
protected void do_OKbtn_actionPerformed(ActionEvent e, String ThisFile) {
Why is this method 'protected'?
Naming convention: 'thisFile', not 'ThisFile'.
if (!ThisFile.equals("")) {
You forgot to check for the parameter being 'null'. This could cause a
'NullPointerException'. Tsk, tsk.
HowManyTab = 0;
ReadAndCombFile(ThisFile); // ?????????????????????
}
}
// ??????{???}???????????????????????????????????????????????????tab??????
String FileInManyLines (String oneline, int CombRow, String Suffix) {
Why is this method package-private?
Naming convention. 'fileInManyLines()'. 'combRow'. 'suffix'.
String PreffixStr; // ????????????????????????????????????????????????????????????
There's only one "f" in "prefix". Don't put the type of the variable in the
name ("Str")!
... [snip] ...
public static void main(String[] args) {
Tidy TideFiles = new Tidy();
}
}
Start GUIs on the Event Dispatch Thread (EDT)!
Do file operations *off* the EDT!
I suggest reading the Swing tutorial and other references about Swing
programming. Also, study the Java coding conventions, around since 1999. Your
code is not thread-safe. Heck, it's not any kind of safe - you leave room for
runtime exceptions, you don't comment the strange parts of your logic (like
your switch on 'Diff' [sic] and the magic values it assumes). Your
pretty-print source is indented incorrectly - how good can your pretty-printer
be if you don't indent correctly in the first place? You don't control the
scope of your variables well ('i' and 'j' in 'strFind()', 'flag' and
'bufferedWriter' in 'stringArrayToFile()'), you don't declare constant those
that should be constants ('TabBlank' [sic]), and you redundantly initialize
variables ('BufferedReader reader = null;', 'int HowManyTab = 0;', etc.). You
declare methods as 'protected' or package-private willy-nilly without
explanation for the choices.
I will venture to guess that you wrote this as an exercise and though you
didn't explicitly say so, posted it here for critique. There are so many Java
pretty-printers out there already. Use one on your own code, why don't you?
Your code needs good formatting, proper naming, corrected access control,
controlled scope, controlled heritability, and the bugs eliminated.
--
Lew
Honi soit qui mal y pense.
http://upload.wikimedia.org/wikipedia/commons/c/cf/Friz.jpg