rks rks - 3 months ago 5
C++ Question

How can I loop through a vector of objects stored within a map? C++

I have a function class with a vector variable that stores other function objects. Each function is mapped in a network class.

std::map<std::string, Function> functions;


When I output a single object it shows the vector variable (
next_func
) of that object after mapping the function.

cout << network.functions.at("C");


Output:

(FUNCTION: id=C, next_func=[D])


However when I try to loop through the map to try and display the vector variable for each mapped function, it show an empty vector variable.

for (auto element : network.functions) {
cout << element.second << "\n";
}


Output:

(FUNCTION: id=C, next_func=[])
(FUNCTION: id=D, next_func=[])
(FUNCTION: id=E, next_func=[])


I'm not sure if it's something to do with my class structures or if I'm not using an appropriate form of iteration for what I need done.

How can I loop through the mapped objects to display/manipulate the objects within the vector variable?

Working example:

Function.h

#ifndef Function_H
#define Function_H

#include <vector>
#include <iterator>
#include <iostream>
#include <string>

using namespace std;

class Function {
public:
string name;
std::vector<Function*> func_pre;
std::vector<Function*> func_next;
public:
Function();
Function(const string& id, int duration);
Function(const Function&);
~Function();
Function& operator=(const Function& t);
string name_() const;
void addNext(Function& s);
void addPre(Function& p);
std::string toString() const;
friend std::ostream& operator<<(std::ostream&, const Function&);
};
#endif


Function.cpp

#include <sstream>
#include "Function.h"

using namespace std;

Function::Function() {
name = "";
}

Function::Function(const string& id, int duration) {
this->name = id;
}

Function::Function(const Function& t) {
this->name = t.name_();
}

Function::~Function() {}

Function& Function::operator=(const Function& t) {
if (this != &t) {
name = t.name;
}
return *this;
}

string Function::name_() const {
return this->name;
}

void Function::addNext(Function& s) {
Function* e = &s;
func_next.push_back(e);
}

void Function::addPre(Function& p) {
Function* e = &p;
func_pre.push_back(e);
}

std::ostream& operator<<(std::ostream& s, const Function& e) {
s << e.toString();
return s;
}

std::string Function::toString() const {
std::string s = "(Function: id=" + name +" ,to=";
s = s+"[";
for(unsigned int i = 0; i < func_next.size(); i++)
s = s + func_next[i]->name_() + " ";
s = s+"], from=[";
for(unsigned int i = 0; i < func_pre.size(); i++)
s = s + func_pre[i]->name_() + " ";
s = s + "])";
return s;
}


Map.h

#ifndef Map_H
#define Map_H

#include <map>
#include <vector>
#include <string>
#include "Function.h"

class Map{

public:
std::map<std::string, Function> fucntions;
public:
explicit Map();
Map(const Map& n);
~Map();
Map& operator=(const Map& i);
void addFunction(const string id, int x);
void addDep(const string& from, const string& to);
std::string toString()const;
};
#endif


Map.cpp

#include "Map.h"
#include <iterator>
#include <iostream>
#include <iterator>

using namespace std;

Map::Map() {
fucntions = {};
}

Map::Map(const Map& n) {
this->fucntions = n.fucntions;
}

Map::~Map() {
}

Map& Map::operator=(const Map& i) {
if (this != &i) {
fucntions = i.fucntions;
}
return *this;
}

void Map::addFunction(const string id, int x) {
Function t(id, x);
fucntions[t.name_()] = t;
}

void Map::addDep(const string& from, const string& to) {
fucntions.at(from).addNext(fucntions.at(to));
fucntions.at(to).addPre(fucntions.at(from));
}

std::string Map::toString() const {
std::string s = "(\n";
std::map<std::string, Function>::const_iterator i = fucntions.begin();
std::map<std::string, Function>::const_iterator end = fucntions.end();
if (i == end)
s += "<Empty>";
else
do{
s += (*i).second.toString();
if (++i != end) s+= ",\n";
}while (!(i==end));
s +="\n)";
return s;
}


main.cpp

#include <iostream>
#include "Map.h"
#include "Function.h"

using namespace std;

int main(){
Map m;
m.addFunction("A", 10);
m.addFunction("B", 30);
m.addFunction("C", 20);
m.addFunction("D", 40);
m.addFunction("E", 20);
m.addDep("A", "B");
m.addDep("A", "C");
m.addDep("B", "D");
m.addDep("D", "E");
m.addDep("C", "E");
m.addDep("B", "C");

cout << m.fucntions.at("C") << "\n\n";

for (auto& element : m.fucntions) {
cout << "Predecessor counts: " << element.first
<< " : "<< element.second << "\n";
}
}

Answer

This copy constructor

Function::Function(const Function& t) {
    this->name = t.name_();
}

… leaves the pointer members with indeterminate values.

With some compilers and options you may get nullpointer values but this is not guaranteed: formally accessing the values of those pointer members is Undefined Behavior.


Additionally, this copy assignment operator,

Function& Function::operator=(const Function& t) {
    if (this != &t) {
        name = t.name;
    }
    return *this;
}

… does not assign the pointer members. Whether that is technically an error or not depends on your logic. But it does look like a bug, because it's an assignment that doesn't preserve the full assigned value.


Tip: instead of initializing members via assignment, you can do that via a constructor's member initializer list, like this:

Function::Function( Function const& other )
    : name( other.name )
    , func_pre( other.func_pre )
    , func_next( other.func_next )
{}

This has the advantage of avoiding extra default-initializations, and it works for members that are non-assignable but copyable.

This particular implementation example is exactly what the compiler generates for you if you just don't define or declare a copy constructor.

Comments