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

Answer 2

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.

Answer 3

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