Simple XML string to user-friendly plain text converter method in Java

Question

I want to write a simple method in Java to convert XML to some user-friendly plain format (like YAML, etc.).

Attributes, null, empty or zero values should not be taken into account. Also, documents with containing mixed content (or comments or processing instructions) should not be parsed/passed into. This should provide the user with a quick overview of the data. Backward conversion is not necessary.

Example:

Input:

        <?xml version="1.0" encoding="UTF-8"?>
        <e>
          <foo>
            <bar>Tari</bar>
          </foo>
          <baz>
            <qux>
              <quux>i gdht</quux>
            </qux>
          </baz>
          <corge>2073</corge>
          <grault>
            <garply>
              <waldo>29856</waldo>
              <fred>13:05:36</fred>
              <plugh>Swaziland</plugh>
            </garply>
          </grault>
        </e>

Output:

        #document:
          e:
            foo:
              bar: Tari
            baz:
              qux:
                quux: i gdht
            corge: 2073
            grault:
              garply:
                waldo: 29856
                fred: 13:05:36
                plugh: Swaziland

Is my recursive docToText() method all right, or is there a smell?

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.xml.sax.SAXException;

public class XMLProcessing {
  public static Document getDocFromXmlString(String xml)
      throws ParserConfigurationException, IOException, SAXException {
    DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
    factory.setValidating(false);
    factory.setIgnoringElementContentWhitespace(true);
    DocumentBuilder builder = factory.newDocumentBuilder();
    return builder.parse(new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8)));
  }

  public static StringBuilder docToText(Node doc, int indent, StringBuilder result) {
    if (doc.hasChildNodes()) {
      if (doc.getChildNodes().getLength() == 1 && !doc.getChildNodes().item(0).hasChildNodes()) {
        String textContent = doc.getTextContent();
        if (textContent != null && !textContent.isBlank() && !"0".equals(textContent)) {
          result
              .append(" ".repeat(indent))
              .append(doc.getNodeName())
              .append(": ")
              .append(textContent)
              .append("\n");
        }
      } else {
        result.append(" ".repeat(indent)).append(doc.getNodeName()).append(":\n");
        NodeList list = doc.getChildNodes();
        for (int i = 0; i < list.getLength(); i++) {
          docToText(list.item(i), indent + 2, result);
        }
      }
    }
    return result;
  }

  public static void test() throws ParserConfigurationException, IOException, SAXException {
    String inputXml =
        """
        <?xml version="1.0" encoding="UTF-8"?>
        <e>
          <foo>
            <bar>Tari</bar>
          </foo>
          <baz>
            <qux>
              <quux>i gdht</quux>
            </qux>
          </baz>
          <corge>2073</corge>
          <grault>
            <garply>
              <waldo>29856</waldo>
              <fred>13:05:36</fred>
              <plugh>Swaziland</plugh>
            </garply>
          </grault>
        </e>
        """;
    String outputText =
        """
        #document:
          e:
            foo:
              bar: Tari
            baz:
              qux:
                quux: i gdht
            corge: 2073
            grault:
              garply:
                waldo: 29856
                fred: 13:05:36
                plugh: Swaziland
        """;
    assert outputText.contentEquals(
        docToText(getDocFromXmlString(inputXml), 0, new StringBuilder()));
  }

  public static void main(String[] args)
      throws ParserConfigurationException, IOException, SAXException {
    test();
  }
}

Answer 1

Proper Naming

The name XMLProcessing is a bit vague, doesn't communicate the responsibility well.

Because the meaning of the verb "processing" is very general (it might imply anything: formatting, validation), it doesn't suggest the intention of converting XML to something else.

XmlConverter will communicate the intent better.

The method name docToText() is misleading, it sounds like it extracts plain text from a document, which is not the case (YAML is a markup language, as well as XML).

Design

You implemented XMLProcessing class as a bag of static methods (aka util class).

Instead, always consider modeling your classes as fully flagged cohesive objects as the first option. This gives you a better idea about the areas of responsibility of this class, and what collaborator-objects it requires. Which leads to cleaner and more testable code ^* .

Did ask yourself why DocumentBuilderFactory has to be instantiated and configured again and again? Why not making it an instance attribute and configure it once?

Also, it's more likely that you'll perform conversion using identical indentation for every document, rather than specifying a different one each time. Hence, it'll be convenient to define a default indentation as another instance attribute, so that specifying indentation will be optional, rather than mandatory (use overloaded methods for that purpose).

Bottom-line: don't just pile methods, leverage the Object-oriented paradigm, otherwise you'll eventually end up with a ball of mud.

^{* Don't get me wrong, just removing static modifiers by itself will not give you a well-designed class. You should be able to recognize classic code smells and be familiar with the basic refactoring technics in order to be able to develop your solution in the iterative fashion evolving the code incrementally.}

By the way, util lasses should define a private constructor to accentuate their role. But I believe that isn't the case when you need a util class.

Class API

The methods that your classes are exposing to the outside world should be well-thought of and convenient to use.

• Don't force the users of the code to make such calls a(b(arg)), give them method c(arg) instead.

I.e. define a hi-level public method performing conversion from XML to YAML.

• And if you don't want your users to dial with org.w3c.dom.Document, then there's no need to expose getDocFromXmlString() and docToText(), instead your class should feature only a public method performing direct conversion.

Recursion

Is my (recursive) docToText() method all right, or is there a smell?

You probably know that Java doesn't support tail recursion (contrary to some other JVM languages like Scala and Kotlin), hence any recursive implementation in Java is not stack-safe.

Usually, XML-documents don't have such a horrible level of nesting that might cause a StackOverflowError, but it doesn't invalidate the point.

Instead, you can implement the Depth-first search iteratively (in case if didn't realize, DFS is what you're doing in your recursive method).

^{Note: that's not a code smell, but a genuine issue.}

Now, regarding smells.

You're forcing the user of the code to provide an empty StringBuilder for now good reason. It only creates a noise, and it's always a good idea to limit the scope of mutation, by keeping it local.

If Java supported tail recursion (so that it would make sense to use recursion to begin with), it would be a better option to define a wrapping method returning a YAML-string which internally calls your recursive method passing an empty StringBuilder.

Tests

• A single test is oftentimes insufficient. There are many missing scenarios.

For instance, you didn't test the conversion of block sequences (which are sometimes called lists). They have a special syntax: we either use square brackets [] around all elements, or prefixing a hyphen - before each item. And I don't see this implemented in your method either.

Or what about comments in the document?

The simple scenarios such an empty document, document with a single empty tag, malformed document are also missing.

They should be there to ensure that behavior is consistent and doesn't change while you're refactoring the code.

• Each test should have a proper name (not just test or test1), preferably augmented by a detailed description which can provided via @DisplayName JUnit annotation.

• Tests should not reside alongside the production code. In a Maven or Gradle project, we have a separate test folder specifically dedicated for that purpose (see standard project structure).

• Use production-grade tools for unit-testing. At the very minimum you need JUnit, preferably in conjunction with a fluent assertions library such as AssertJ, or Hamcrest.