代码重构：代码的坏味道

2019独角兽企业重金招聘Python工程师标准>>>

今天在向maven仓库发文件，干不了什么活，于是就在osc中转转，转到共享代码里，看到一篇
Java 解析/读取XML 文件  @老枪
于是就用这个例子来典型分析一下。
原文如下：

1. [代码]students.xml     

<?xml version="1.0"?>
<students>
    <student>
        <name>John</name>
        <grade>B</grade>
        <age>12</age>
    </student>
    <student>
        <name>Mary</name>
        <grade>A</grade>
        <age>11</age>
    </student>
    <student>
        <name>Simon</name>
        <grade>A</grade>
        <age>18</age>
    </student>
</students>

2. [代码]XMLParser.java     

package net.viralpatel.java.xmlparser; 
  
import java.io.File; 
import javax.xml.parsers.DocumentBuilder; 
import javax.xml.parsers.DocumentBuilderFactory; 
  
import org.w3c.dom.Document; 
import org.w3c.dom.Element; 
import org.w3c.dom.Node; 
import org.w3c.dom.NodeList; 
   
public class XMLParser { 
   
    public void getAllUserNames(String fileName) { 
        try { 
            DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); 
            DocumentBuilder db = dbf.newDocumentBuilder(); 
            File file = new File(fileName); 
            if (file.exists()) { 
                Document doc = db.parse(file); 
                Element docEle = doc.getDocumentElement(); 
   
                // Print root element of the document 
                System.out.println("Root element of the document: "
                        + docEle.getNodeName()); 
   
                NodeList studentList = docEle.getElementsByTagName("student"); 
   
                // Print total student elements in document 
                System.out.println("Total students: " + studentList.getLength()); 

                if (studentList != null && studentList.getLength() > 0) { 
                    for (int i = 0; i < studentList.getLength(); i++) { 
  
                        Node node = studentList.item(i); 
   
                        if (node.getNodeType() == Node.ELEMENT_NODE) { 
   
                            System.out.println("====================="); 

                            Element e = (Element) node; 
                            NodeList nodeList = e.getElementsByTagName("name"); 
                            System.out.println("Name: "
                                    + nodeList.item(0).getChildNodes().item(0) 
                                            .getNodeValue()); 
  
                            nodeList = e.getElementsByTagName("grade"); 
                            System.out.println("Grade: "
                                    + nodeList.item(0).getChildNodes().item(0) 
                                            .getNodeValue()); 
   
                            nodeList = e.getElementsByTagName("age"); 
                            System.out.println("Age: "
                                    + nodeList.item(0).getChildNodes().item(0) 
                                            .getNodeValue()); 
                        } 
                    } 
                } else { 
                    System.exit(1); 
                } 
            } 
        } catch (Exception e) { 
            System.out.println(e); 
        } 
    } 
    public static void main(String[] args) { 
 
        XMLParser parser = new XMLParser(); 
        parser.getAllUserNames("c:\\students.xml"); 
    } 
}

类中存在的问题有：

1.类名起得不好--给人的感觉你这个解析的类，结果是个测试解析的类 
2.方法名起得不好--给人的感觉是有返回值的，结果是个没有返回值的 
3.方法内容写得太乱，把解析及数据获取啥的都放在一个方法里了。 
4.代码逻辑性不好，前面写了System.out.println("Total students: "  + studentList.getLength());后面还在写studentList != null，如果前面不出异常，这里就没有用；如果前面出异常，这里检查点根本就过不来。 
5.中间用了System.exit，显然这不是一个好的处理，人家调你个参数，你给悄无声息的把整个应用停止了。 
6.代码重复率太高 

NodeList nodeList = e.getElementsByTagName("name"); 

                            System.out.println("Name: " 
                                    + nodeList.item(0).getChildNodes().item(0) 
                                            .getNodeValue()); 
                            nodeList = e.getElementsByTagName("grade"); 
                            System.out.println("Grade: " 
                                    + nodeList.item(0).getChildNodes().item(0) 
                                            .getNodeValue()); 
                            nodeList = e.getElementsByTagName("age"); 
                            System.out.println("Age: " 
                                    + nodeList.item(0).getChildNodes().item(0) 
                                            .getNodeValue());

这个太难看了。 

小结：这段代码把许多不良编程习惯都带出来了。

当然也可能是@老枪 当时只是随便写写的。

当然有人或许会问，你怎么写？换我写，我的写法是这样的，当然类名我这里只是用来测试一下，因此写个TestXmlParser ，实际上应该是两个类的：一个功能类，一个测试的类：

public class TestXmlParser {
    public static void main(String[] args) throws Throwable {
        File file = new File("E:/test/students.xml ");
        XmlStringParser parser = new XmlStringParser();
        XmlDocument document = parser.parse(IOUtils.readFromInputStream(
                new FileInputStream(file), "utf-8"));
        printStudents(document.getRoot());
    }
    private static void printStudents(XmlNode studentsNode) {
        for(XmlNode studentNode:studentsNode.getSubNodes("student")){
            printStuent(studentNode);
        }
    }
    private static void printStuent(XmlNode studentNode) {
        printSubTagByName(studentNode,"name");
        printSubTagByName(studentNode,"grade");
        printSubTagByName(studentNode,"age");
    }
    private static void printSubTagByName(XmlNode studentNode,String tagName) {
        System.out.println( studentNode.getSubNode(tagName).getContent());
    }
}

当然了，上面没有做边界检查，仅是为了说明例子而已，但是执行结果与@老枪 的是一样的。。

转载于:https://my.oschina.net/tinyframework/blog/193864