Rigidness and verbose nature of XML parsing

Question

I've been developing a simple console application in C++ using the TinyXML2 library, and I honestly can't help but feel what I'm doing is not very robust. Maybe that's just the nature of parsing XML (this is my first foray into the topic, so I've no idea.)

The following code works to the best of my knowledge, but it feels like there has to be a better way than manually navigating to each element, checking for null pointers, then grabbing the information. Is there a more elegant way to handle this? I have to be missing something within the documentation, because I feel like I'm writing a lot of redundant code here.

#include <iostream>
#include "tinyxml2.h"

int main() {
    tinyxml2::XMLDocument document;

    if (document.LoadFile("games.xml") == tinyxml2::XML_SUCCESS) {
        std::cout << "'games.xml' successfully loaded." << std::endl;

        tinyxml2::XMLElement *root = document.RootElement();

        std::cout << root->Value() << std::endl;

        tinyxml2::XMLElement *child;

        for (child = root->FirstChildElement(); child != nullptr; child = child->NextSiblingElement()) {
            std::cout << "\t" << child->Value() << std::endl;

            if (child->FirstChildElement("name") == nullptr) {
                std::cerr << "Error: null pointer." << std::endl;
                return 1;
            }
            if (child->FirstChildElement("system") == nullptr) {
                std::cerr << "Error: null pointer." << std::endl;
                return 1;
            }
            if (child->FirstChildElement("status") == nullptr) {
                std::cerr << "Error: null pointer." << std::endl;
                return 1;
            }
            const char* game_name = child->FirstChildElement("name")->GetText();
            const char* game_system = child->FirstChildElement("system")->GetText();
            const char* game_status = child->FirstChildElement("status")->GetText();

            if (game_name == nullptr || game_system == nullptr || game_status == nullptr) {
                std::cout << "Error: null pointer." << std::endl;
                return 1;
            }
            std::cout << "\t\t" << game_name << std::endl;
            std::cout << "\t\t" << game_system << std::endl;
            std::cout << "\t\t" << game_status << std::endl;
        }
    } else {
        std::cerr << "Unable to load 'games.xml'. Error code: " << document.ErrorID() << std::endl;
        return 1;
    }
    return 0;
}

Answer 1

In your code I do agree there more redundancy than I like to see. I would suggest writing some helper functions, and perhaps leveraging exceptions instead of return codes (I won't show that here).

const char* FirstChildElementText(tinyxml2::XMLElement *parent, const char *child_name)
{
    auto child = parent->FirstChildElement(child_name);
    return (child != nullptr) ? child->GetText() : nullptr;
}
: : :
for (child = root->FirstChildElement(); child != nullptr; child = child->NextSiblingElement()) {
    auto game_name = FirstChildElementText(child, "name");
    auto game_system = FirstChildElementText(child, "system");
    auto game_status = FirstChildElementText(child, "status");

    if (game_name == nullptr || ...)
    : : :
}

This helps a bit. Always be on the lookout for a good refactoring. Even if you already found a better XML library for your needs.

(I have things I dislike about both DOM and SAX style XML parsing, either being too lenient or too much work. Really, all in all, I suspect that directly working with XML is approaching things from the wrong level. But I don't have a great solution, so I'll get off my soap box.)

Answer 2

You could use "\n" instead of std::endl to help a bit with performance. The latter also flushes the buffer, which you probably don't need here (even then, you could just use "\n" and std::flush). More info about the differences here.

You could also make the return error values more readable with EXIT_FAILURE.

Answer 3

I write an XSD file to define the structure of the XML: for example, to specify that the XML contains an element named name which contains text.

Before I parse the XML at run-time, I invoke one validation function, to verify that the XML has the structure defined by the XSD.

I then use simplified parsing statements which assume (because I just already tested/validated it against the XSD) that the XML has the required/specified format: for example, I would read child->FirstChildElement("name")->GetText(); without checking for null pointers.

---

However, maybe the TinyXML-2 library you're using doesn't support validation using XSD files (i.e. doesn't implement a 'validate' method).

The library's README includes code like the following:

// Navigate to the title, using the convenience function,
// with a dangerous lack of error checking.
const char* title = doc.FirstChildElement( "PLAY" )->FirstChildElement( "TITLE" )->GetText();

I don't know what "dangerous" is meant to mean in that context. I wonder whether it would be enough to wrap that code in an O/S-specific exception handler.