代码重构：代码的坏味道

原文如下：

1. [代码]students.xml     

?

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18

<? 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     

?

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

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