patterncppMinor
Rigidness and verbose nature of XML parsing
Viewed 0 times
rigidnessnaturexmlparsingandverbose
Problem
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.
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
#include "tinyxml2.h"
int main() {
tinyxml2::XMLDocument document;
if (document.LoadFile("games.xml") == tinyxml2::XML_SUCCESS) {
std::cout Value() FirstChildElement(); child != nullptr; child = child->NextSiblingElement()) {
std::cout Value() FirstChildElement("name") == nullptr) {
std::cerr FirstChildElement("system") == nullptr) {
std::cerr FirstChildElement("status") == nullptr) {
std::cerr 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;
}Solution
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).
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.)
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.)
Code Snippets
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 || ...)
: : :
}Context
StackExchange Code Review Q#26598, answer score: 3
Revisions (0)
No revisions yet.