Parse an XML file using objects / methods

Question

So I spent a while this morning creating an xml parser in java which is part of a job interview. I would love to have some people tear it apart so I can learn from any mistakes that I made so I can grow for future job interviews.

XML file:

<Employees>
 <Employee>
 <Name> First Last</Name>
 <ID> 00001 </ID>
</Employee>
<Employee>
 <Name> First2 Last</Name>
 <ID> 00002 </ID>
</Employee>
<Employee>
 <Name> First3 Last</Name>
 <ID> 00003 </ID>
</Employee>
</Employees>

Java File (126 LOC)-

//@Author HunderingThooves
import java.io.File;
import java.util.Scanner;
import java.util.ArrayList;
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;
import org.xml.sax.SAXException;
import org.xml.sax.SAXParseException;
public class Main {
 static class openXML{
 private int employeeNumber;
 private String employeeName;
 void getEmployee(int i){
 String[] xmlStr = xmlLoader(i);
 setEmployeeName(xmlStr[0]);
 setEmployeeNumber(xmlStr[1]);
 }
 void setEmployeeNumber(String s){
 employeeNumber = Integer.parseInt(s);
 }
 void setEmployeeName(String s){
 employeeName = s;
 }
 String getEmployeeName(){
 return employeeName;
 }
 int getEmployeeNumber(){
 return employeeNumber;
 }
 };
 public static final String[] xmlLoader(int i){
 String xmlData[] = new String[2];
 try {
 int employeeCounter = i;
 DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance();
 DocumentBuilder docBuilder = docBuilderFactory.newDocumentBuilder();
 Document doc = docBuilder.parse (new File("test.xml"));
 // normalize text representation
 doc.getDocumentElement ().normalize ();
 NodeList listOfEmployee = doc.getElementsByTagName("Employee");
 Node firstEmployeeNode = listOfEmployee.item(employeeCounter-1);
 int totalEmployees = listOfEmployee.getLength();
 //Break xml file into parts, then break those parts down int an array by passing individual elements to srtings
 if(firstEmployeeNode.getNodeType() == Node.ELEMENT_NODE){
 Element firstEmployeeElement = (Element)firstEmployeeNode;
 //-------
 NodeList nameList = firstEmployeeElement.getElementsByTagName("Name");
 Element nameElement = (Element)nameList.item(0);
 NodeList textNameList = nameElement.getChildNodes();
 xmlData[0]= (((Node)textNameList.item(0)).getNodeValue().trim()).toString();
 //-------
 NodeList IDList = firstEmployeeElement.getElementsByTagName("ID");
 Element IDElement = (Element)IDList.item(0);
 NodeList textIDList = IDElement.getChildNodes();
 xmlData[1]= (((Node)textIDList.item(0)).getNodeValue().trim()).toString();
 //------
 }//
 }
 catch(NullPointerException npe){
 System.out.println("The employee number you searched for is incorrect or does not yet exist, try again. ");
 String s[] = {" ", " "};
 main(s);
 }
 catch (SAXParseException err) {
 System.out.println ("** Parsing error" + ", line " 
 + err.getLineNumber () + ", uri " + err.getSystemId ());
 System.out.println(" " + err.getMessage ());
 }
 catch (SAXException e) {
 Exception x = e.getException ();
 ((x == null) ? e : x).printStackTrace ();
 }
 catch (Throwable t) {
 t.printStackTrace ();
 }
 return xmlData;
 }
 public static void main(String args[]){
 openXML myXmlHandler = new openXML();
 Scanner input = new Scanner(System.in);
 int i = -1;
 do{
 try{
 String s = "";
 System.out.println("Enter the employee number that you're searching for: ");
 s = input.next(); 
 try{
 i= Integer.parseInt(s); 
 } catch(NumberFormatException nfe){ i = -1;}
 } catch(Exception e){System.out.println("Error: " +e.getMessage());}
 }while(i <= 0);
 myXmlHandler.getEmployee(i);
 System.out.println("The employee Name is: " + myXmlHandler.getEmployeeName());
 System.out.println("The employee Number is: " + myXmlHandler.getEmployeeNumber());
 System.exit(0);
 } 
}

There is one known issue in the file that I will not mention unless someone finds it, I spent about an hour trying to pin it down and the best I could do is sweep it under the rug.

Answer 1

I just did some refactoring to your code and came up with below output.

Few points to be mentioned,

1. Lets try to use OOPs concept
2. Follow naming convention (also includes meaningful names for classes and methods)
3. Why catch NPE (do validate your input and avoid such scenario) ?
4. Try to avoid unnecessary codes (here there is no use of "System.exit(0);")
5. The parsing logic can be fine tuned but this is already good for a sample program

Between good try :) And good luck with your interview.

public class XMLParserExample {
static class Employee {
 private final int id;
 private final String name;
 public Employee(int id, String name) {
 this.id = id;
 this.name = name;
 }
 public int getId() {
 return id;
 }
 public String getName() {
 return name;
 }
}
static class EmployeeXMLParser {
 private final Document document;
 public EmployeeXMLParser(String fileName) {
 document = loadXml(fileName);
 }
 Employee findEmployee(int index) {
 NodeList listOfEmployee = document.getElementsByTagName("Employee");
 Node employeeNode = listOfEmployee.item(index - 1);
 if (employeeNode != null && employeeNode.getNodeType() == Node.ELEMENT_NODE) {
 String name = getElementValue((Element) employeeNode, "Name");
 String id = getElementValue((Element) employeeNode, "ID");
 return new Employee(Integer.parseInt(id), name);
 }
 return null;
 }
 private String getElementValue(Element parentElement, String elementName) {
 NodeList nodeList = parentElement.getElementsByTagName(elementName);
 Element element = (Element) nodeList.item(0);
 NodeList childNodes = element.getChildNodes();
 return (((Node) childNodes.item(0)).getNodeValue().trim()).toString();
 }
 private Document loadXml(String fileName) {
 DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance();
 try {
 DocumentBuilder docBuilder = docBuilderFactory.newDocumentBuilder();
 Document doc = docBuilder.parse(new File(fileName));
 doc.getDocumentElement().normalize();
 return doc;
 } catch (ParserConfigurationException e) {
 throw new IllegalStateException("Error parsing the XML", e);
 } catch (SAXException e) {
 throw new IllegalStateException("Error parsing the XML", e);
 } catch (IOException e) {
 throw new IllegalStateException("Error accessing the XML", e);
 }
 }
};
void start() {
 EmployeeXMLParser employeeParser = new EmployeeXMLParser("test.xml");
 Scanner input = new Scanner(System.in);
 boolean successful = false;
 do {
 System.out.println("Enter the employee number that you're searching for: ");
 try {
 Employee employee = employeeParser.findEmployee(Integer.parseInt(input.next()));
 if (employee == null) {
 printTryAgain();
 } else {
 System.out.println("The employee Number is: " + employee.getId());
 System.out.println("The employee Name is: " + employee.getName());
 successful = true;
 }
 } catch (NumberFormatException nfe) {
 printTryAgain();
 }
 } while (!successful);
}
private void printTryAgain() {
 System.out.println("The employee number you searched for is incorrect or does not yet exist, try again.");
}
public static void main(String args[]) {
 new XMLParserExample().start();
} }

• HunderingThooves: This was a really great answer and your code is very helpful. As for the system.exit(0); It was actually because if it was removed I would get a NumberFormatException...I was wondering if you could explain why that was happening with my code? I spent a solid 30 minutes trying to figure it out.. I could catch it but I couldn't trace it.
• Sridhar G: Strange that you got NFE. I couldn't figure out what went wrong. All I see is even if you don't use it in your program it is going to be terminated. Additional info: Never use System.exit in programs unless it is needed. System.exit normally used in batch programs to indicate whether the process/program was successfully completed. See System.exit()
• HunderingThooves: Try entering in a bad value (using my xml file try entering in a large number, 123451, then enter a working value when it loops.) and it will NFE without the system.exit. It's weird.

Answer 2

Why do you have two different try blocks here? Why not join them?

do{
 try{
 String s = "";
 System.out.println("Enter the employee number that you're searching for: ");
 s = input.next(); 
 try{
 i= Integer.parseInt(s); 
 } catch(NumberFormatException nfe){ i = -1;}
 } catch(Exception e){System.out.println("Error: " +e.getMessage());}
}while(i <= 0);

Answer 3

I have done:

Employees emp = JAXB.read(Employees.class, filename);

where read was:

@SuppressWarnings("unchecked") public static <T> T read(
 Class<T> t, String filename) throws Exception {
 Preconditions.checkNotNull(filename);
 Preconditions.checkNotNull(t);
 java.io.File f = checkNotNull(new java.io.File(filename));
 JAXBContext context = JAXBContext.newInstance(t);
 Unmarshaller u = context.createUnmarshaller();
 return (T) u.unmarshal(f);
}

You can also read from stream, string or ...

I still don't like what I have written.

They are not judging if you can write code, that works. They want to see a proof that it works (tests), what has been done (jdoc documentation) and readable code.