DOM parser implemented from scratch

Question

I have implemented a DOM parser. I would like any comments for optimizing, improving, and making the code coherent to best coding practices.

final class Node {

    private final String nodeName;
    private final String nodeValue;
    private final NodeTypeEnum nodeType;
    private final Map<String, String> attributeMap;
    private List<Node> childNodeList;

    public Node(String nodeName, String nodeValue, NodeTypeEnum nodeType) {
        this(nodeName, nodeValue, nodeType, new HashMap<String, String>());
    }

    public Node(String nodeName, String nodeValue, NodeTypeEnum nodeType, Map<String, String> attributeMap) {
        if (nodeType == null) {
            throw new NullPointerException("Node type cannot be null.");
        }
        if (attributeMap == null) {
            throw new NullPointerException();
        }
        this.nodeName = nodeName;
        this.nodeValue = nodeValue;
        this.nodeType = nodeType;
        this.attributeMap = attributeMap;
    }

    // basically a Tag
    public String getNodeName() {
        return nodeName;
    }

    // the value of element ie between the ankle brackets.
    public String getNodeValue() {
        return nodeValue;
    }

   // node type: now thinkong if this needs 
    public NodeTypeEnum getNodeType() {
        return nodeType;
    }

    public String getAttribute(String name) {
        return attributeMap.get(name);
    }

    public List<Node> getChildNodes() {
        return childNodeList;
    }

    /**
     * Returns only elements-nodes, not the text-nodes
     */
    public List<Node> getElementsByTagName(String tagName) {
        final List<Node> listNode = new ArrayList<Node>();
        populateElementsByTagName(getChildNodes().iterator(), tagName, listNode);
        return listNode;
    }

    private void populateElementsByTagName(Iterator<Node> itr, String tagName, List<Node> listNode) {
        assert itr != null;
        assert tagName != null;
        assert listNode != null;

        // tags with same name are on the same level thus if-else is a good optimization.
        while (itr.hasNext()) {
            final Node node = itr.next();
            /**
             * http://stackoverflow.com/questions/629029/using-getters-within-class-methods/629038#629038
             */
            if (node.getNodeType() == NodeTypeEnum.TEXT_NODE) continue;

            if (node.getNodeName().equals(tagName)) {
                listNode.add(node);
            } else {
                populateElementsByTagName(node.getChildNodes().iterator(), tagName, listNode);
            }
        } 
    }

    public String getTextContent() {
        return populateTextContext(getChildNodes().iterator()).toString();
    }

    private StringBuilder populateTextContext(Iterator<Node> itr) {
        final StringBuilder sb = new StringBuilder();
        while (itr.hasNext()) {
            Node n = itr.next();

            if (n.nodeType == NodeTypeEnum.TEXT_NODE) {
                sb.append(n.nodeValue + "\n");
            } else {
                sb.append(populateTextContext(n.getChildNodes().iterator()).toString());
            }
        }
        return sb;
    }

    public void setChildNodeList(List<Node> childNodeList) {
        this.childNodeList = childNodeList;
    }
}

public class DOMScratch {
    private final String filePath;
    private Node root;

    public DOMScratch(String filePath) {
        if (filePath == null) {
            throw new NullPointerException("File path cannot be null");
        }
        this.filePath = filePath;
    }

    // we want to throw IO exception, and not continue further processing.
    public void pareXMLDoc() throws IOException {
        BufferedReader br = null;
        try {
            br = new BufferedReader(new FileReader(filePath));
            root = readAndParse(br).get(0);
        }  finally {
                br.close();
        }
    }

    /**
     * In essence a DFSish algo.
     */
    private List<Node> readAndParse(BufferedReader br) throws IOException {
        final List<Node> nodeList = new ArrayList<Node>();
        String line = null;

        while ((line = br.readLine()) != null) {
            final Node node = parseLine(line);
            if (node == null) { return nodeList; }  // last tag encountered.
            nodeList.add(node);
            if (node.getChildNodes() != null) { continue; } // all textnodes, which are child of an element should be at same level.
            node.setChildNodeList(readAndParse(br)); // is this better design ?  
        }
        return nodeList;
    }

    private Node parseLine(String line) throws IOException {
        assert line != null;

        String trimmedLine = line.trim();
        char[] ch = trimmedLine.toCharArray();
        if (ch[1] == '/') {return null; }

        // <City coast=East>NewYorkCity</City>
        String[] subElement = line.split("<");  // str[1] = City>SanDiego, and str[2] = /City> which is to be discarded.
        /**
         * subElement[1] = City coast=East>NewYork
         * subElement[2] = /City>
         */
        String[] tagAttributeText = subElement[1].split(">");
        /**
         * tagAndAttributeMap[0] = City coast="East"
         * tagAndAttributeMap[1] = NewYork
         */
        String[] tagAttribute = tagAttributeText[0].split(" ");
          /**
           * tagAttribute[0] = City
           * tagAttribute[1] = coast="East"
           */
        final Map<String, String> attributeMap = new HashMap<String, String>();
        Node node;
        if (tagAttribute.length > 1) {
            String[] keyValue = tagAttribute[1].split("=");
            attributeMap.put(keyValue[0], keyValue[1]);
            node = new Node(tagAttribute[0], null, NodeTypeEnum.ELEMENT_NODE, attributeMap);
        } else {
            node = new Node(tagAttribute[0], null, NodeTypeEnum.ELEMENT_NODE);
        }

        if (tagAttributeText.length > 1) {
            List<Node> nodeList = new ArrayList<Node>();
            nodeList.add(new Node(null, tagAttributeText[1], NodeTypeEnum.TEXT_NODE));
            node.setChildNodeList(nodeList);
        }
        return node;
    }

    public Node getRootElement() {
        return root;
    }

    public static void main(String[] args) {
        DOMScratch dom = new DOMScratch("/Users/ameya.patil/Desktop/xml.txt");
        try {
            dom.pareXMLDoc();
        } catch (IOException e) {
            e.printStackTrace();
        }
        Node node = dom.getRootElement();
        System.out.println(node.getNodeName());
        System.out.print(node.getTextContent());
        System.out.println(node.getAttribute("rank"));
        List<Node> nodeList = node.getElementsByTagName("NewYork");
        for (Node nl : nodeList) {
            System.out.println(nl.getElementsByTagName("City").get(0).getTextContent());
        }
    }
}

Answer 1

Do not put the word Enum in an enum type name. NodeTypeEnum should be just NodeType. The constants of that enum should not have NODE in their names; the class name already implies that each constant represents a type of node.

Javadoc is for packages, classes, methods and fields. Multi-line comments inside code should start with /*, not /**.

There are StringBuilder.append methods for all reference and primitive types. Calling toString() on an object before passing it to StringBuilder.append is redundant, regardless of the object's type.

Unless you specifically want the library to work with Java versions older than Java 7, use try (BufferedReader br = new BufferedReader(…)) so br is closed automatically and safely. Your pareXMLDoc [sic] method, as written, would throw a NullPointerException if the file were not found, because br would still be null. The try-with-resources syntax is the easy way to remove any such concerns.

I realize your main method is just for testing, but ignoring the IOException is not helpful. If a document can't be parsed, you don't want to continue, you want to stop. Either wrap the IOException in a RuntimeException, or declare main with throws IOException.

Answer 2

You've tagged it "XML" so I suppose your intent is to parse anything that conforms to XML 1.0? If not, you should say what language you are intending to parse. At the moment, you're not remotely close to parsing general XML, for example you'll fail to handle "<" in a CDATA section or comment, or " " within an attribute value.

Frankly, it looks to me as if you have no knowledge at all of parser writing, and are trying to work it out from first principles. The result looks rather like my first attempt to build a brick wall, that is to say, a total bodge. I would strongly suggest you get some basic computer science textbooks and read them, or enrol on a suitable course.

Sorry to be so brutal. Asking for criticism is the first sign of a good programmer, so you'll get there in the end.