I had a bi-directional object association implemented with raw pointers and it worked without flaw. Then I decided to refactor my code with smart pointers and all of a sudden a string member (depName) of one of the classes (Department) is no longer accesssible after object initialization. I checked if the parameter gets passed correctly and in the constructor everything is ok. The member gets the intended value. But afterwards it is no longer accessible and in addition to that the code that ran smoothly before, now crashes. I have no clue why.
EDIT: The same seems to be happening to the string variable name used for in the Manager class. END EDIT
This is my code (sorry it's a lot - I reduced it as much as I dared):
in the header files:
class Manager;
class Department
{
private:
string depName;
shared_ptr<Manager> head;
vector <shared_ptr<Manager>> depMembers;
public:
Department(string depName, shared_ptr<Manager> head);
virtual ~Department();
string getDepName() const;
void setDepName(const string &value);
void addMember(shared_ptr<Manager> newMember);
void removeMember(shared_ptr<Manager> who);
const shared_ptr<Manager>getHead() const;
void setHead(shared_ptr<Manager>value);
double sumOfIncome();
void show();
};
//--------------------------------
class Department;
class Manager
{
private:
string name;
float salary;
float bonus;//Bonus ist ein Prozentsatz
weak_ptr<Department> myDepartment;
// Department * myDepartment; //With this raw pointer the code still worked*
public:
Manager(string, float, float);
virtual ~Manager();
float income()const ;
string toString()const ;
double calcBonus() const;
shared_ptr<Department> getMyDepartment() const;
void setMyDepartment(shared_ptr<Department> abt);
float getSalary() const;
string getName() const;
};
in the cpp files: department.cpp
//---------------------------------------------------
Department::Department(string depName, shared_ptr<Manager>head)
:depName(depName),head(nullptr)
{
this->setHead(head);
cout << "\nIn constructor parameter depName: " + depName;
cout << "\n instancevariable " + this->depName << endl;
}
//--------------------------------
Department::~Department()
{}
//--------------------------------
string Department::getDepName() const
{
return this->depName;
}
//--------------------------------
void Department::setDepName(const string &value)
{
depName = value;
}
//--------------------------------
void Department::addMember(shared_ptr<Manager> newMember)
{
depMembers.push_back(newMember);
}
//--------------------------------
void Department::removeMember(shared_ptr<Manager> who)
{
vector<shared_ptr<Manager>>::iterator itMember = depMembers.begin();
//Iterator must be dereferenced to access data
while( *itMember != who){
itMember++;
}
if( *itMember == who)
depMembers.erase( itMember);
}
//--------------------------------
const shared_ptr<Manager> Department::getHead() const
{
return head;
}
//--------------------------------
void Department::setHead(shared_ptr<Manager>value)
{
if( head != nullptr && head->getMyDepartment()!= nullptr)
head->setMyDepartment(nullptr);//department of old head is deleted
//new head of department assigned
head = value;
//bidirektionaler access
if(head !=nullptr)
head->setMyDepartment( shared_ptr<Department>(this));
}
//--------------------------------
double Department::sumOfIncome()
{
double sum = 0;
for(unsigned int i=0; i < depMembers.size(); i++){
sum += depMembers[i]->getSalary() ;
}
return sum;
}
//--------------------------------
void Department::show()
{
cout <<"----------------" << endl;
cout << "Department: " << this->depName << " run by " << head->getName()<<endl;
cout <<"----------------" << endl;
cout << "Members: " << endl;
cout <<"----------------" << endl;
cout << head->toString() << endl;
for( unsigned int i=0; i < depMembers.size() ; i++){
cout <<"----------------" << endl;
cout << depMembers[i]->toString()<< endl;
}
cout <<"----------------" << endl;
}
manager.cpp
//---------------------
float Manager::getSalary() const
{
return salary;
}
//----------------------------------
string Manager::getName() const
{
return name;
}
//----------------------------------
Manager::Manager(string n, float s, float bon)
:name(n),salary(s), bonus(bon)
{}
//----------------------------------
Manager::~Manager(){}
//----------------------------------
float Manager::income()const
{
return (salary + calcBonus() );
}
//----------------------------------
string Manager::toString() const
{
stringstream ss;
ss << name << "\n heads the department ";
shared_ptr<Department> dep = myDepartment.lock();
if( dep !=nullptr)
ss<< dep->getDepName();
else ss << " NONE ";
ss << "\nBonus: " << calcBonus();
ss << "\nIncome: " << income();
return ss.str();
}
//----------------------------------
double Manager::calcBonus()const
{
shared_ptr<Department> dep = myDepartment.lock();
if(dep != nullptr)
return dep->sumOfIncome()* bonus;
else
return 0;
}
//----------------------------------
shared_ptr<Department> Manager::getMyDepartment() const
{
// if( !meineAbteilung->expired())
return myDepartment.lock();
}
//----------------------------------
void Manager::setMyDepartment( shared_ptr<Department> dep)
{
myDepartment = dep;
}
//----------------------------------
test run:
int main(){
shared_ptr<Department> itDepartment
= make_shared<Department>("IT",make_shared<Manager>("Julia", 66066, 0.15));
itDepartment->show();
return 0;
}
I could tag John Zwinck's answer as correct - (thank you John!), but I myself needed more information to understand the issue, so I have decided to post my own answer.
There are 2 problems with my approach:
shared_ptr should never be made from raw pointer-variables, because each initialization of a shared_ptr from a raw pointer will create a new manager-object with its own reference count. An existing manager-object will only get used if a new shared_ptr is created from one of the shared_ptrs or weak_ptrs of the existing manager-object. (Somehow the book I read managed to omit this important bit of information.) this is a raw pointer, so in my constructor of Department a second manager-object is created. The reference count of the second manager-object decreases to 0 as soon as the constructor is left - deleting the object I just finished creating. A shared_ptr should therefore never be created from this itself. It's a fatal flaw.enable_shared_from_this, which supplies some template magic to make a shared_ptr from this in the background. Unfortunately shared_from_this() is not available in the constructor, because the object-build is not finished. The conclusion is, that an object cannot pass a reference to itself another object in the constructor. There is just no way to get that to work. A workaround, calling setHead() directly after the constructor is the way out. Coming from Java, where passing this from the constructor is the norm, this is something you just have to live with...Apart from John Zwinck nudging me in the right direction, I found a paper from David Kieras very helpful: http://www.umich.edu/~eecs381/handouts/C++11_smart_ptrs.pdf