Reputation: 994
I am trying to do a relatively simple task here and am not sure how to go about it.
I have two classes of interest Parent
and Child
. Both of them are a special case of the class Human. I create them as such.
Relevant bit of main.cpp:
int main() {
//create an object of class Parent parent1 with a name Albert.
Parent parent1("Albert");
//create an object of a class Child child1 named John.
Child child1("John");
return 0;
}
The Parent.h header includes a function called addChild()
. What it does is simply adds a child to a vector of Children inside a Parent object.
Relevant bit of Parent.h:
class Parent: public Human {
public:
vector<Human> Children;
...
void addChild(Human aHuman){
Children.push_back(aHuman);
}
};
What I would like to achieve is for the addChild()
function to simultaneously add a parent to the child as well. That is, to do something like:
void addChild(Human aHuman) {
Children.push_back(aHuman);
aHuman.Parent = MYSELF;
}
Above, MYSELF denotes the bit that I do not know how to code up.
So when I run this:
int main() {
//create an object of class Parent parent1 with a name Albert.
Parent parent1("Albert");
//create an object of a class Child child1 named John.
Child child1("John");
parent1.addChild(child1);
return 0;
}
I would like Albert to be set as a parent of John and John as a child of Albert.
Can anyone help me with this?
EDIT:
The code as per Jan's suggestion:
#include <iostream>
#include<vector>
class Human{
};
class Child;
class Parent : public Human
{
std::vector<Child> children;
public:
friend Child;
void addChild(Child& child)
{
children.emplace_back(child);
child.parents.emplace_back(*this);
};
};
class Child : public Human
{
std::vector<Parent> parents;
public:
friend Parent;
void addParent(Parent& parent)
{
parents.emplace_back(parent);
parent.children.emplace_back(*this);
}
;
};
Errors:
In member function 'void Parent::addChild(Child&)':
invalid use of incomplete type 'class Child'
child.parents.emplace_back(*this);
error: forward declaration of 'class Child'
class Child;
Upvotes: 0
Views: 3146
Reputation: 5039
I fear that you have a XY Problem and mixing up important concepts of object polymorphism and C++ design in general but, here is a solution to what you asked:
#include <vector>
#include <memory>
struct Human {
virtual ~Human() = default;
};
struct Parent;
struct Child: Human{
Child(std::string) {}
void addParent(std::weak_ptr<Parent> aParent) {
Parent = aParent;
}
private:
std::weak_ptr<Parent> Parent;
};
struct Parent: Human, std::enable_shared_from_this<Parent> {
Parent(std::string) {}
void addChild(Child aChild){
aChild.addParent(shared_from_this());
Children.push_back(aChild);
}
private:
std::vector<Child> Children;
};
int main() {
auto parent1 = std::make_shared<Parent>("Albert"); //create an object of class Parent parent1 with a name Albert.
Child child1("John");//create an object of a class Child child1 named John.
parent1->addChild(child1);
}
It ain't pretty, but thats more to the problems of your design i.e. question.
Note that this code is still far from optimal with all this call by value, copying, lifetime and irrelevance of the Human
class. However better solutions would need more context and design.
Update on Update
Update on Jahns code
Jahns code is not valid from C++11 to C++14:
As you asked whats wrong with jans code: It does not compile because it will not work in general. As std::vector<T>
requires a complete type, and is basically equivalent as storing a copy of T
.
Since C++17:
Jahns code should run, but because now std::vector<T>
can also work as storing a reference to T
, basically equivalent to the tricks described below.
Because of your design you have 2 classes with a circular dependency: Childs should save their parents and parents their childs. So you i.e. Jan decided to save a copy of parent in the child and a copy of the child in parent. But in order to save it, a computer has to know how big it is.
So child has to know how big a parent is to know how big itself will be, but in order to know how big a parent is, it has to know first how big itself is, because its also saved in parent. See the problem? Self reference is a devils playground and usually leads to infinite recursion.
However a trick to break the cycle is, to just say to child, "don't save the whole parent, just a reference to it, so you don't need to know how big it is". This is done by the forward deceleration of parent, and the usage of smart pointers, which safe you from doing bad things, like trying to use a blank reference. (Thats way raw pointers like Parent*
are bad, don
t use them, you probably will get undefined behavior, i.e. seg faults on fridays 2 function calls late)
Another very similar common trick would be that child and parent agreed on the same root, like a human base class. It also solves the problem by using references. You human class suggests you first tried or your teacher urged you to go this way.
But all those tricks increase complexity and lower performance and safety, because you pushed work from compile time to runtime. Java, C#, Python and Co. are even doing this per default. Makes anti patterns more common IMHO.
I can only repeat: I recommend to read a book on C++, C++ Primer by Lippman is what I read as a beginner.
Upvotes: 3
Reputation: 860
You encountered a common problem of novice OOP programmers: You try to solve a relational problem by extending OOP classes, instead of creating a relational class.
The novice approach
First, let's continue with your approach for a bit... Of course, each Parent
has a Child
and each Child
has a Parent
, and the natural way seems to store these relations inside each class. Thus:
class Parent : public Human
{
std::vector<Child> children;
public:
void addChild(const Child& child);
};
class Child : public Human
{
std::vector<Parent> parents;
public:
void addParent(const Parent& parent);
};
Now, let's implement the add...
functions:
void Child::addParent(const Parent& parent)
{
parents.emplace_back(parent);
}
void Parent::addChild(const Child& child)
{
children.emplace_back(child);
}
As you already mentioned, this requires you to update the parent and the child separately. So, let's change our approach and make the classes friends:
class Child;
class Parent : public Human
{
std::vector<Child> children;
public:
friend Child;
void addChild(Child& child);
};
class Child : public Human
{
std::vector<Parent> parents;
public:
friend Parent;
void addParent(Parent& parent);
};
This friend relation can now be used to update the other class as well:
void Child::addParent(Parent& parent)
{
parents.emplace_back(parent);
parent.addChild(*this);
}
void Parent::addChild(Child& child)
{
children.emplace_back(child);
child.addParent(*this);
}
Damn, this does not work! What happened? We effectively created an infinite loop. Because adding a Child
to a Parent
adds the child, but also calls the addParent
function on the child, which then adds the parent, but also has to call the addChild
function on the parent, which adds a child, ... you get the point.
How can we fix that? One way is to add the child/parent another way:
void Child::addParent(Parent& parent)
{
parents.emplace_back(parent);
parent.children.emplace_back(*this);
}
void Parent::addChild(Child& child)
{
children.emplace_back(child);
child.parents.emplace_back(*this);
}
This works, but we have to pay a heavy cost for that. The Parent
and the Child
have to know the internals of each other to work properly. This means, you cannot easily change these classes, and it is hard for users to extend them. (They need to study your whole class internals to be sure that it works.)
Furthermore, there are several other problems with this approach:
addParent
or addChild
function? (Hint: We save a copy of the object without the proper elements added.)Moreover, we are using copies of the objects inside the classes right now. This means, that updating a parent will leave the actual data inside the child untouched (and vice-versa). This can partly be solved by using pointers, but also means that you have to begin managing pointers for this simple task...
The method approach
So, what is a proper solution? One easy option, is a functions which deals with these interconnected updates. Taking the first implementation (without updating both parent and child), we can use the following function:
void linkChildToParent(Parent& parent, Child& child)
{
parent.addChild(child);
child.addChild(parent);
}
The linkChildToParent
function...
Child
and Parent
to extend.This approach solves most of our problems and can even be extended to deal with pointers properly and very easily. Yet, we still have to put some work into updating the right members, ... How is this fixed? By proper OOP of course!
The OOP approach
The problems we are having are all related to the way we store the relation between a parent and its child. This is an indicator, that our abstraction is wrong! Thinking about it, we can come up with the right abstraction, a ParentalRelation
class:
//Given, that we only have basic functionality, a struct is sufficient here...
struct ParentalRelation
{
Child child;
Parent parent;
};
This allows us to store parent-child information in a separate table. Furthermore:
Child
and Parent
do not need to know nothing about their internals.In addition, a switch to pointers is easily done because only ParentalRelation
is needed to manage them. This has the advantage of always having the right information, when either the parent or child is changed, without additional update code.
Disclaimer
ParentalRelation
approach can be very difficult in practice.Upvotes: 1
Reputation: 217283
You have mainly 2 issues:
std::vector<Human>
only store Human part of Parent
/Child
.Upvotes: 2
Reputation: 240
you have an issue when you using void addChild(Human aHuman) without & you need to do void addChild(Human& aHuman) if you what the changed to save in the object without & you copy 2 times the objects 1 in the call to function and one when you insert to addChild(child1); MYSELF can be this but you need to change the vector not to get objects
Upvotes: 0