Kivis
Kivis

Reputation: 994

C++ Interaction between two classes in a function

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

Answers (4)

Superlokkus
Superlokkus

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

jan.sende
jan.sende

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:

  • What happens if we change the line order in the addParent or addChild function? (Hint: We save a copy of the object without the proper elements added.)
  • How can you remove a child from a parent and update the parents of the child properly?
  • How can you be sure as a parent, that the child does not change your internal behaviour in a weird way, or vice-versa?

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...

  • will update your child and your parent correctly.
  • does not depend on line ordering.
  • is easy to read and maintain.
  • is easy for users of 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.
  • Extending any of the classes is easy.
  • Deleting or changing parents/children is very easy.
  • It is frankly much less code to understand and to write.

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

  • I hope, this was somewhat clear.
  • While it might seem easy here, finding the ParentalRelation approach can be very difficult in practice.

Upvotes: 1

Jarod42
Jarod42

Reputation: 217283

You have mainly 2 issues:

  • You pass by value instead of reference, so you only modify a copy
  • You have object slicing: std::vector<Human> only store Human part of Parent/Child.

Upvotes: 2

alon
alon

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

Related Questions