Path: csiph.com!usenet.pasdenom.info!news.albasani.net!.POSTED!not-for-mail From: Lew Newsgroups: comp.lang.java.programmer Subject: Re: I develop a Java program to format Java codes Date: Fri, 02 Mar 2012 09:25:27 -0800 Organization: albasani.net Lines: 177 Message-ID: References: <5690138.193.1330687781011.JavaMail.geo-discussion-forums@ynca15> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Trace: news.albasani.net 4IqfcfkX3m/1S35iZfDh/47KH6dOswjE02rB3BZ8qvUuEw44+3mm4fO8fheH/OtZmk9XIm0t0le29PLyMYALegXHrShFGgMODM/EhV5bDjifeLjCn7qixYPjztr0hFvC NNTP-Posting-Date: Fri, 2 Mar 2012 17:25:29 +0000 (UTC) Injection-Info: news.albasani.net; logging-data="6ZbXyDJnqdK5iieaDq+s5PmQAa805c09j+41XIZfFNuqxmiUtBuao2lWAewLaeUn2jshS34QXjwLA3OEcdh2k3DUON/K9sV8KeQNpMkEGx8tocR9U2nWCQdRYEgWA4Kt"; mail-complaints-to="abuse@albasani.net" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.2) Gecko/20120216 Thunderbird/10.0.2 In-Reply-To: <5690138.193.1330687781011.JavaMail.geo-discussion-forums@ynca15> Cancel-Lock: sha1:ECGVTGFAzT0bW5O+8We54WxuMTY= Xref: csiph.com comp.lang.java.programmer:12587 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