代码重构：代码的坏味道

原文如下：

1. [代码]students.xml

1

2

3

4

5

6

7

8

9

10

11

12

13

14

15

16

17

18

2. [代码]XMLParser.java

1

2

3

4

5

6

7

8

9

10

11

12

13

14

15

16

17

18

19

20

21

22

23

24

25

26

27

28

29

30

31

32

33

34

35

36

37

38

39

40

41

42

43

44

45

46

47

48

49

50

51

52

53

54

55

56

57

58

59

60

61

62

63

64

65

66

67

68

69

70

71

 
            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.代码重复率太高

1

2

3

4

5

6

7

8

9

10

11

12

13

 
          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 ，实际上应该是两个类的：一个功能类，一个测试的类：

1

2

3

4

5

6

7

8

9

10

11

12

13

14

15

16

17

18

19

20

21

22

 
          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()); 
         
          } 
         
          }

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