James Willson James Willson - 22 days ago 6
C++ Question

No viable overloaded '=' on a c++ program

I have some C++ code to find the differences in xml and print, using a map to rename the node tags. This is the full code:

#include "pugi/pugixml.hpp"

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

int main()
{
// Define mappings, default left - map on the right
const std::map<std::string, std::string> tagmaps
{
{"id", "id"}, {"description", "content"}
};

pugi::xml_document doca, docb;
pugi::xml_node found, n;
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("entry")) {
const char* id = node.child_value("id");
mapa[id] = node;
}

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

for (auto& ea: mapa) {
std::cout << "Removed:" << std::endl;
ea.second.print(std::cout);
// CURL to remove entries from ES
}

for (auto& eb: mapb) {
// change node name if mapping found
found = tagmaps.find(n.name());
if((found != tagmaps.end()) {
n.set_name(found->second.c_str());
}
}

}


This is the error I get when I try and compile. I'm new to C++ and I'm having a hard time fixing it. Any help or input would be appreciated.

src/main.cpp:49:8: error: no viable overloaded '='
found = tagmaps.find(n.name());

Answer

Note: this is more of an extended comment than a direct answer to the question you asked.

At least if I've divined the intent correctly, this code:

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

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

...is intended to produce the intersection of the two groups of objects into mapa and the difference of the two groups in mapb. That being the case, I'd rather express those intents a little more directly:

auto by_id = [](pugi::xml_node const &a, pugi::xml_node const &b) {
    return strcmp(a.get_child("id"), b.get_child("id")) == 1;
};

auto const &a = doca.child("data").children("entry");
auto const &b = docb.child("data").children("entry");

std::set_intersection(a.begin(), a.end(), 
                      a.begin(), b.end(),
                      std::inserter(mapa, mapa.end()),
                      by_id);

std::set_difference(a.begin(), a.end(),
                    b.begin(), b.end(),
                    std::inserter(mapb, mapb.end()),
                    by_id);

Although marginally longer, I think this expresses the intended result enough more clearly to easily justify the extra length.

As for the part that was originally causing you difficulty:

for (auto& eb: mapb) {
    // change node name if mapping found
    found = tagmaps.find(n.name());
    if((found != tagmaps.end()) {
    n.set_name(found->second.c_str());
    }
}

I'll admit I'm rather confused as to what you really even intend this to accomplish. You iterate across mapb, but never use eb inside of the loop at all. At the same time, you do a search for n.name(), but you haven't initialized n to actually contain anything first.

I'm going to guess you really intended to include something like:

auto &n = eb.second;

...as the first statement inside the loop (in which case you apparently don't need to define the existing n outside the loop at all).

With that in place, we see that part of what you have in tagmaps is apparently redundant--having the {"id", "id"} entry doesn't change the name of those entries, so there's no real point in having it there at all.

That leaves us with:

const std::map<std::string, std::string> tagmaps {
    {"description", "content"}
};

// ...

for (auto& eb: mapb) {
    auto &n = eb.second;
    auto found = tagmaps.find(n.name());
    if((found != tagmaps.end()) 
        n.set_name(found->second.c_str());
}

At least to me, that looks like it at least stands some chance of doing something at least marginally useful. This could be changed to use std::transform, but I doubt you gain much (if anything) but doing so.