3
\$\begingroup\$

This is my code which works and compiles but I feel it may be non-optimal, especially the tag mapping and use of the if command.

It reads two .xml files and finds what has been added or deleted on the new file. For new items it uses a map to link a tag name to a category of data (content tag is a description for example). This is important since different files have different naming conventions on the tags.

Finally, it does work on the data depending on its category (coming soon) and prints it.

#include "pugi/pugixml.hpp"

#include <iostream>
#include <string>
#include <map>

int main()
{

    std::string var;

    const std::map<std::string, std::string> tagMap {
        {"id", "id"}, {"description", "content"}, {"url", "web_address"}
    };

    pugi::xml_document doca, docb;
    std::map<std::string, pugi::xml_node> mapa, mapb;

    if (!doca.load_file("a.xml") || !docb.load_file("b.xml")) { 
        std::cout << "Can't find input files";
        return 1;
    }

    for (auto& node: doca.child("data").children("item")) {
    const char* id = node.child_value("id");
    mapa[id] = node;
    }

    for (auto& node: docb.child("data").children("item")) {
    const char* idcs = node.child_value("id");
        if (!mapa.erase(idcs)) {
        mapb[idcs] = node;
        }
    }

     for (auto& eb: mapb) {
        // Loop the tagMap, and try and associate the tag name to the content
         for (auto& kv : tagMap) {
             if (kv.first == "id") {
             std::string var = eb.second.child_value(kv.second.c_str());
             // Do work on returned value of id tag in the future (I.e validation)
             std::cout << var << endl;
             }
             if (kv.first == "description") {
             std::string var = eb.second.child_value(kv.second.c_str());
             // Do work on returned value of content tag in the future
             std::cout << var << endl;
             }
             if (kv.first == "url") {
             std::string var = eb.second.child_value(kv.second.c_str());
             // Do work on returned value of web_address tag in the future
             std::cout << var << endl;
             }
         }
     }
}
\$\endgroup\$
3
\$\begingroup\$

I will only comment about the following not-fully-implemented part.

 for (auto& kv : tagMap) {
     if (kv.first == "id") {
     std::string var = eb.second.child_value(kv.second.c_str());
     // Do work on returned value of id tag in the future (I.e validation)
     std::cout << var << endl;
     }
     if (kv.first == "description") {
     std::string var = eb.second.child_value(kv.second.c_str());
     // Do work on returned value of content tag in the future
     std::cout << var << endl;
     }
     if (kv.first == "url") {
     std::string var = eb.second.child_value(kv.second.c_str());
     // Do work on returned value of web_address tag in the future
     std::cout << var << endl;
     }
 }

Even without much code filling the blanks, it seems that you want to have some associative tag => function data. In which case, you could create an std::unordered_map with strings as keys and associating function pointers to them, generated from captureless lambdas. A small example:

std::unordered_map<std::string, void(*)(const std::string&)> tasks = {
    { "id", [](const std::string& val) {
        // Do work on returned value of id tag
    }},
    { "description", [](const std::string& val) {
        // Do work on returned value of content tag
    }},
    { "url", [](const std::string& val) {
        // Do work on returned value of web_address tag
    }}
};

for (auto& kv: tagMap) {
    auto it = tasks.find(kv.first);
    if (it != tasks.end()) {
        it->second(eb.second.child_value(kv.second.c_str()));
    }
}

That way, your code is easy to extend to other tags by representing tasks associated to tags as data and storing them into a map. Now, it will only work if you don't need captures, otherwise, you may have to use std::function or some more heavyweight mechanism instead of a big chain of if.

\$\endgroup\$
  • \$\begingroup\$ Thank you very much for the help. Would you mind explaining what you mean by "need captures" please \$\endgroup\$ – Jimmy Apr 21 '15 at 14:22
  • 1
    \$\begingroup\$ @Jimmy A lambda capture corresponds to a variable in the body of a lambda that comes from the outer scope. For a C++ lambda [](){}, the capture corresponds to what is in the square brackets []. A lambda with a capture cannot be converted to a function pointer. \$\endgroup\$ – Morwenn Apr 21 '15 at 14:25
  • \$\begingroup\$ I'm sorry to be a pain but within the "do work on id" bit how do I access the id, for example if I want to just print the id? \$\endgroup\$ – Jimmy Apr 22 '15 at 10:08
  • 1
    \$\begingroup\$ @Jimmy I guess that when kv.first == "id", then the id you're referring to is kv.second, right? In this case, it will be fed to it->second which is the corresponding lambda, so the id would be the lambda val parameter. \$\endgroup\$ – Morwenn Apr 22 '15 at 11:33
  • \$\begingroup\$ thank you for the reply. In this case can I get the value with the code you posted above or is this the case where I would need the more heavyweight method like std::function. I'm just trying to get a description where I actually have access to what was found in the tag, so std::cout << it for example would print 1 (the id) \$\endgroup\$ – Jimmy Apr 22 '15 at 11:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.