Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is passing(in constructor) a pointer to class that contains it a bad design and if so what is the solution

often I encounter code like

/*initializer list of some class*/:m_member(some_param,/* --> */ *this)

Reason why this is done is so that m_member can call member functions from the class that contains it... aka

//code in class that is m_member instance of

    m_parent->some_function();

I personally dislike it because I consider it pathetic design("dear child do you know what are you doing to your class encapsulation"), but I would like to know is in general this behavior bad, and if so how to avoid this kind of design.

EDIT: please dont focus on this in initalizer list, lets say it is in ctor body.

like image 470
NoSenseEtAl Avatar asked Dec 19 '12 17:12

NoSenseEtAl


1 Answers

It can be disastrous, since your parent is not constructed a the time of the reference-set. The following example will demonstrate this:

#include <iostream>
using namespace std;

struct TheParent;

struct TheChild
{
    TheChild(TheParent& parent);
    TheParent& myParent;
};

struct TheParent
{
    TheParent()
      : mychild(*this)
      , value(1)
    {
        cout << "TheParent::TheParent() : " << value << endl;
    }

    TheChild mychild;
    int value;
};

TheChild::TheChild(TheParent& parent)
   : myParent(parent)
{
    cout << "TheChild::TheChild() : " << myParent.value << endl;
};

int main()
{
    TheParent parent;
    return 0;
}

Produces the following output, clearly noting the indeterminate state of the parent object:

TheChild::TheChild() : 1606422622
TheParent::TheParent() : 1

Bottom line: don't do it this way. You would be better served to use a dynamic child allocation instead, but even this has caveats:

#include <iostream>
using namespace std;

struct TheParent;

struct TheChild
{
    TheChild(TheParent& parent);
    TheParent& myParent;
};

struct TheParent
{
    TheParent()
      : mychild(NULL)
      , value(1)
    {
        mychild = new TheChild(*this);
        cout << "TheParent::TheParent() : " << value << endl;
    }

    ~TheParent()
    {
        delete mychild;
    }

    TheChild* mychild;
    int value;
};

TheChild::TheChild(TheParent& parent)
   : myParent(parent)
{
    cout << "TheChild::TheChild() : " << myParent.value << endl;
};


int main()
{
    TheParent parent;
    return 0;
}

This give you what you're likely hoping for:

TheChild::TheChild() : 1
TheParent::TheParent() : 1

Note, however, even this has issues if TheParent is an intermediate class in an inheritance chain, and you're desiring to access potentially overridden virtual implementations of functions in derived classes that have yet to be constructed.

Again, bottom line, if you find yourself doing this, you may want to think about why you need to in the first place.

like image 178
WhozCraig Avatar answered Oct 29 '22 17:10

WhozCraig