Prevent temporary from extending its lifetime?
Asked Answered
G

2

8

This may be impossible, but I was wondering if it was possible to keep a temporary from ever lasting past its original expression. I have a chain of objects which point to parent objects, and a member function which will create a child object, a simplified example is here

class person{
    string name;
    person * mommy;
public:
    person(const string & nam, person * m = 0) : name(nam), mommy(m) {}
    person baby(const string & nam){
        return person(nam, this);
    }
    void talk() const{
        if (mommy) mommy->talk();
        cout << name << endl;
    }
};

int main(){
    person("Ann").baby("Susan").baby("Wendy").talk();     // fine

    const person & babygirl = person("Julie").baby("Laura"); // not fine

    babygirl.talk();    // segfault
    return 0;
}

The way I want to use person is to pass it to a function, and something like this:

void use(const person & p) {
    p.talk();
}
use(person("Anna").baby("Lisa"));

Is fine.

This will work fine as long as none of the temporaries survive past the original expression, but if I bind one of the final temporaries to a const reference, its parents don't survive, and I get a segfault. I can hide person's copy constructor and assignment operator, but is there any way I can prevent this kind of error from happening? I'd like to avoid dynamic allocation if possible.

Gynecoid answered 27/9, 2011 at 20:11 Comment(2)
Note that this code is "not fine" in the same way that writing const int &i = std::vector<int>(1)[0]; is "not fine". vector doesn't stop you writing that, and doesn't need to. The key here is that because destroying the mommy renders the baby unusable, the baby is part of the mommy. That's what's wrong with the design, it's counter-intuitive. You're trying to patch that up by preventing there being any such thing as an orphan, which might be appropriate but you should also consider whether orphans should just have better defined behavior, or should be more obviously a bad thing to create.Itin
Agreed with Steve: this is just bad design showing. The appearance of a naked pointer should have given away that something's off.Eichhorn
C
3

It looks like you are creating a data structure here where children have pointers to their parents. Using temporaries is guaranteed to cause you grief in this case. In order to make this safe, you will need to dynamically allocate and possibly use some sort of reference counting.

Have you considered using boost::shared_ptr? It is an implementation of a reference counted smart pointer class. Using shared_ptr and perhaps some factory methods, you might be able to get the effect you want and reduce the pain of dynamic memory allocation. I tried it out and it seems to work. Once the code exits the scope, the objects are all destroyed because there are no references left to the shared_ptrs.

Edit: In response to zounds' comment, I have modified the example so that the root object controls the data structure's lifetime.

#include <iostream>
#include <string>
#include <vector>
#include <boost\shared_ptr.hpp>
#include <boost\weak_ptr.hpp>

using boost::shared_ptr;
using boost::weak_ptr;

using std::string;
using std::cout;
using std::endl;
using std::vector;

class person;
typedef shared_ptr<person> Person;
typedef weak_ptr<person> PersonWk;

class person {    
    PersonWk pThis;
    friend Person makePerson(const string & nam, Person m = Person());

    string name;
    PersonWk mommy; // children should not affect parent lifetime, so store weak ptr
    vector<Person> children; // parents affect children lifetime so store child shared ptrs

    // make constructor private so that you can only make a person using factory method
    person(const string & nam, Person m) : name(nam), mommy(m) 
    { 
        // for demo purposes
        printf("creating %s\n", nam.c_str());
        ++personCount; 
    }

    // undefined copy constructor and assignment operators
    person(const person&);
    person& operator=(const person&);

public:
    // for demo purposes
    static int personCount;

    ~person() 
    { 
        // for demo purposes
        printf("destroying %s\n", name.c_str());
        --personCount; 
    }

    Person baby(const string & nam){        
        Person child = makePerson(nam, Person(pThis));
        children.push_back(child);
        return child;
    }

    void talk() const{
        if (Person mom = mommy.lock()) 
            mom->talk();
        cout << name << endl;
    }
};

int person::personCount = 0;

// factory method to make a person
Person makePerson(const string & name, Person m) {
    Person p = Person(new person(name, m));
    p->pThis = p; // stash weak_ptr so I can use it to make a shared_ptr from "this" in the baby method
    return p;
}

void use(const Person p) {
    printf("In method use...\n");
    p->talk();
}

int _tmain(int argc, _TCHAR* argv[])
{
    printf("personCount=%d\n", person::personCount);
    {
        Person ann = makePerson("Ann");

        // ann has baby and grandbaby, pass grandbaby to use method
        use(ann->baby("Susan")->baby("Wendy"));

        ann.reset(); // remove reference from root object. Destruction ensues...
    }
    printf("personCount=%d\n", person::personCount);
    return 0;
}
Ciborium answered 27/9, 2011 at 20:23 Comment(3)
Thanks for the answer. The problem for me with shared ptr is that the root object may need to have a specific destruction point, and I don't want to worry that it there might be a stored pointer to it somewhere. I only need the temporaries to exist for as long as a function call, and then they can die. I'm using a child->parent structure where the children aren't allowed to affect their parents (except through the normal public interface everyone is allowed to use)Gynecoid
One approach could be to store the root object at a known location. Then have each parent keep shared_ptrs to their children and the children could keep weak_ptrs to their parents. Then when you set the root object to NULL (thereby removing all references), then the tree is destroyed from top to bottom.Ciborium
Modified code sample to use the approach in my previous comment.Ciborium
R
0

You'll have to do something like this:

void use(const person & p) {
    p.talk();
}
person a("Anna");
use(a.baby("Lisa"));

That way, the parent "a" doesn't go out of scope until you're truly done with it (after the call to "use").

The problem with the original code is that the parent "Anna" only has to stay around long enough to call "baby" and the parent can be discarded before making the function call. By making the parent a variable with scope, you can control when it's destructed.

Does this look dangerous to me? Yes. Therefore I'd suggest a look at m-sharp's answer on dynamic allocation. But if you wanting a method that doesn't need reference counting, etc. then you can do it this way...

Rigatoni answered 27/9, 2011 at 20:29 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.