Retrieve data from an XML file in Java

Question

few months back I did a code test for a company and I quickly managed to solve it within 5-10 minutes. But, I got rejected because it wasnt "good enough".

What is good enough? they said its mainly how I wrote the code and that I was missing exception handling..

Anyways I was supposed to retrieve data from an XML file and store it into a file. Here is my code:

 public static void main(String[] args) throws ParserConfigurationException, IOException, SAXException {
        xmlDataToFile("src/resources/xml-File.xml", "41");
    }

    static void xmlDataToFile(String url, String value) throws ParserConfigurationException, IOException, SAXException {
        DocumentBuilder documentBuilder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
        Document document = documentBuilder.parse(url);
        NodeList nodeList = document.getElementsByTagName("target");
        for (int i = 0; i < nodeList.getLength(); i++){
            Node node = nodeList.item(i);
            if (node.getParentNode().getAttributes().item(0).getTextContent().equals(value)){
                Files.write(Paths.get("textFile.txt"), Collections.singleton(nodeList.item(i).getTextContent()));
            }
        }

How can I improve this ?

The objective is to only save the text content from last node, or also keep old? — Elikill58
– Elikill58, Commented Apr 12, 2023 at 8:13
@Elikill58 The objective is to only save the content, nothing else. — Con101
– Con101, Commented Apr 12, 2023 at 8:23
IF they gave no expectations AND did not explain "not good enough" THEN my cynical side says they had a certain person in mind for the job and were only going through the motions of "interviewing" others. — radarbob
– radarbob, Commented Apr 13, 2023 at 0:05
Throwing exceptions from main is a really bad idea. The exceptions, if thrown, should be cached and handled inside main. — convert
– convert, Commented Jun 8, 2023 at 9:55

Michael Kay · Accepted Answer · 2023-04-12 09:03:17Z

Well, there's a problem here, which is that your question is being read here by people who know the subject far better than your assessors; so you're not actually asking for what the experts consider to be best practice, but for what your assessors, who aren't experts, consider to be best practice.

Many experts will tell you not to use DOM for this. 90% of Java programmers probably would use DOM (as you are doing), but there are far better libraries available (for example JDOM2 or XOM). The only reason anyone uses DOM is because they haven't bothered to look at alternatives. But your assessors are probably expecting a DOM solution.

Now the only thing that strikes me as really bad about your code (and it's hard to judge without seeing the specification of the input XML) is

getParentNode().getAttributes().item(0).getTextContent().equals(value)

because that's looking at the first attribute of the parent of the current target element, and the order of attributes is unpredictable. Even if you know that the parent element will always have exactly one attribute, it's best not to make that assumption in your code. I would expect to see you accessing a specific attribute of the parent element by name, for example

value.equals(getParentNode().getAttibute("name"))

which is also written so it doesn't fail if the attribute is absent.

Another thing I'm not keen on in your answer is the repeated opening of the file in

Files.write(Paths.get("textFile.txt")

That feels thoroughly inefficient to me, though I've no idea if it's really a problem or just my imagination.

Personally, and I have no idea whether your assessors would be satisfied with the answer, I would do this with a one-line XPath 3.1 expression. For example, using the Saxon API because it's the one I know best:

   Processor proc = new Processor(false):
   XPathCompiler xp = proc.newXPathCompiler();
   XdmItem sourceDoc = proc.newDocumentBuilder().build(    
      new StreamSource("src/resources/xml-File.xml"));
   XdmItem result = xp.evaluate(
      "string-join(//target[../@name='42'], '\n')", 
      sourceDoc);
   
   Files.write(Paths.get("textFile.txt"), result.getStringValue());

Thank you Michael, and yes your comment makes sense. The thing is, they didn't care much about what library I used, but rather the code that I wrote. When I used DOM, I explained to them the downside of using DOM since it uses a lot of space which is unnecessary (and provided other better options such as SAX). But either way, they just didn't like my code. Your code looks cleaner I guess. Thank you! — Con101
– Con101, Commented Apr 12, 2023 at 10:44

Stack Exchange Network

Retrieve data from an XML file in Java

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Retrieve data from an XML file in Java

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions