Sign in to follow this  

[C++] Automatic generation of create()/destroy() functions

This topic is 3286 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Every now and then I like to post stuff like this just to see it get torn apart. So what do we have this time? This is a class that enables you to have control over when an object is created and destroyed without using dynamic memory allocation. For example, if you have an object that doesn't have a default constructor, you can't have an instance of it as a class data member. You have to use a pointer along with new/delete (or just new, if you're using a smart pointer). The class is called Holder (not a great name, I know). Here's the implementation: Holder.h
#ifndef HOLDER_H
#define HOLDER_H

#include <cassert>


// Note: Currently supports objects with zero, one or two parameter c'tors
template <class T>
class Holder {
public:
    Holder():created(false)
    {
    }


    ~Holder() {
        destroy();
    }


    Holder(const Holder<T> &other) {
        create(other);
    }


    Holder<T> & operator=(const Holder<T> &other) {
        if (this == &other) {
            return *this;
        }

        // Not sure if the asserts are a good idea...
        assert(created);
        assert(other.created);

        getObj() = other.getObj();
        return *this;
    }

    
    void destroy() {
        if (created) {
            created = false;
            getObj().~T();
        }
    }


    T* operator->() {
        assert(created);
        return &getObj();
    }


    const T* operator->() const {
        assert(created);
        return &getObj();
    }


    T & operator*() {
        assert(created);
        return getObj();
    }


    const T & operator*() const {
        assert(created);
        return getObj();
    }
    
    
    void create() {
        new (obj) T;
        created = true;
    }


    template <class P1>
    void create(const P1 &p1) {
        new (obj) T(p1);
        created = true;
    }


    template <class P1, class P2>
    void create(const P1 &p1, const P2 &p2) {
        new (obj) T(p1, p2);
        created = true;
    }

private:
    T& getObj() {
        return *(reinterpret_cast<T*>(obj));
    }


    const T& getObj() const {
        return (const_cast<Holder<T>*>(this))->getObj();
    }

    char obj[sizeof(T)];
    bool created;
};


#endif  // HOLDER_H
Here's a little test:
#include "Holder.h"

#include <iostream>
#include <string>
using namespace std;


struct Student {
    string name;
    int grade;

    Student(const string &name, int grade)
        : name(name), grade(grade)
    {
        cout << "Student::Student() for " << name << endl;
    }


    ~Student() {
        cout << "Student::~Student() for " << name << endl;
    }


    void print() const {
        cout << name << ", " << grade << endl;
    }
};


void main() {
    Holder<Student> st1;
    Holder<Student> st2;

    cout << "Hello" << endl;

    st1.create("John", 87);
    st2.create("Mike", 92);

    st1->print();
    (*st2).print();
    st2 = st1;
    st2->print();

    st1.destroy();
    st1.destroy();  // Does nothing

    // No call to st2.destroy();

    cout << "Goodbye" << endl;
}
I'd like to hear your comments about the usefulness and correctness of this class. I'm just posting this out of boredom, so feel free to be as brutal as you like.

Share this post


Link to post
Share on other sites
Quote:

For example, if you have an object that doesn't have a default constructor, you can't have an instance of it as a class data member. You have to use a pointer along with new/delete (or just new, if you're using a smart pointer).

That sounds like your motivation for making this "holder". And it doesn't sound right at all. You just use the initializer lists (you already use them in your code) to init the object in question. This is perfectly legal. And 'A' has no default constructor.


class A
{
public:
A(int,int){}

};

class B
{
public:
B():a(1,1){}
private:
A a;
};

...
B b;





Quote:

st1.destroy();
st1.destroy(); // Does nothing

// No call to st2.destroy();

Code like that is just begging for errors. Why not destroy st2? how would I know that? how can i keep track of that? Someone WILL forget at somepoint what the code is supposed to do, ESPECIALLY when someone just copy pastes it.


Containers are tricky. Holder<myClass> will break if I overload operator & to do something other than return the address of the object in question.



Overall, I'm sorry I just don't see the usefulness of this "holder" class at all.

Share this post


Link to post
Share on other sites
Quote:
Original post by KulSeran
That sounds like your motivation for making this "holder". And it doesn't sound right at all. You just use the initializer lists


I meant something like this:


class A {
public:
A(int) {}
};


class B {
public:
B() {
cout << "Enter data" << endl;
int data;
cin >> data;
a(data); // Obviously doesn't compile
}

private:
A a;
;}



Basically it's for cases where you must perform some set of operations before creating the object.

Quote:
Quote:

st1.destroy();
st1.destroy(); // Does nothing

// No call to st2.destroy();

Code like that is just begging for errors. Why not destroy st2?


This is just to ensure that the object will be destructed, even if you forget to call destroy(). Another option might be to trigger an assert in the destructor, but I don't know if that's a good idea.

Share this post


Link to post
Share on other sites
The life time of objects in C++ is defined precisely. An object that is successfully created should be an operational object, or shouldn't have been created at all (which mostly translate into: if the execution reaches a point past the creation of an object, then the object exists. Otherwise an exception have been thrown).

Your system allows an object to be in a state where it is created, only it really isn't. That's an horrible thing to do.

One very powerful programming pattern in c++ (which isn't possible with the other contemporary popular languages) is RAII, explained there: http://www.hackcraft.net/raii/

This doesn't work when you muddy up the object life duration semantics with additional initialization and destruction methods.

Doing such things is one of several reasons why MFC sucked.

Share this post


Link to post
Share on other sites
Quote:
Original post by KulSeran
Containers are tricky. Holder<myClass> will break if I overload operator & to do something other than return the address of the object in question.


I'm not sure what you mean, but isn't that true for any class? For example with code like this:


class A {
public:
const A & operator=(const A &other) {
if (this == &other) // <--
// ...
}
};


Quote:
Original post by swiftcoder
Your copying semantics are very, very strange - I wouldn't recommend using this as is.


Yeah, I wasn't sure how to handle that...

Quote:
However, what does you holder class offer which cannot be matched by std::auto_ptr (or std::tr1::shared_ptr, if auto_ptr isn't enough)?


Like I said, it doesn't use dynamic memory allocation.

Quote:
Original post by Zlodo
Your system allows an object to be in a state where it is created, only it really isn't. That's an horrible thing to do.


I agree, but I tried to make sure that if you use the holder before properly creating it, an assertion will be triggered.

Quote:
One very powerful programming pattern in c++ (which isn't possible with the other contemporary popular languages) is RAII, explained there: http://www.hackcraft.net/raii/

This doesn't work when you muddy up the object life duration semantics with additional initialization and destruction methods.


Well, the holder will destruct its internal object even if you don't call destroy(), so in that sense it's kind of like RAII... Or did I misunderstand you?

Share this post


Link to post
Share on other sites
Quote:

class A {
public:
A(int) {}
};


class B {
public:
B() {
cout << "Enter data" << endl;
int data;
cin >> data;
a(data); // Obviously doesn't compile
}

private:
A a;
;}


OBJECT CREATION is NOT a place for DATA INPUT.
Sorry. But that is REALLY bad practice right there. You should be inputing data outside the constructor.
and even if you don't fix that, most objects that don't have a default constructor have reasons for it. You should work with it, not against it.

Share this post


Link to post
Share on other sites
Quote:
Original post by KulSeran
OBJECT CREATION is NOT a place for DATA INPUT.
Sorry. But that is REALLY bad practice right there. You should be inputing data outside the constructor.
and even if you don't fix that, most objects that don't have a default constructor have reasons for it. You should work with it, not against it.


Maybe it was a bad example, but like I said, this is for situations where you have to create the object after doing several operations so you can't use the initializer list. In fact, my holder allows you to do this for objects that don't have default constructors.

BTW, although it's unrelated, could you please explain why you shouldn't input data inside a constructor?

Share this post


Link to post
Share on other sites
Quote:
Original post by Gage64
Well, the holder will destruct its internal object even if you don't call destroy(), so in that sense it's kind of like RAII... Or did I misunderstand you?


What happens if, after you instantiate the object but before you initialize it, you encounter an error condition and need to exit the function, or an exception is thrown?

(oops, didn't notice the created flag.)

Share this post


Link to post
Share on other sites
I think the RAII post is more to the point of:
I have a mutex assistance class called ScopeLock.
Lock doesn't have a default constructor, you have to call ScopeLock(mutex).
The scoping rules for C++ insure once it is created it holds the mutex and once it dies the mutex is released.
If you get fancy and do "new ScopeLock(mutex)" you atleast know you have to smart pointer or manage the destruction.

{
Holder<ScopeLock> L;
// ... A
{
L.create(mutex);
// ... B
}
// ... C
}




Now, tell me it is 100% clear where ScopeLock's scope begins and ends.
Is A locked? B? C? This is a coding nightmare.
Besides, you lose (same with new ScopeLock(mutex)) the auto-magic of knowing the lock is released no mater how the function ends up leaving the scope level it was created for.

Share this post


Link to post
Share on other sites
Quote:
Original post by Gage64
... this is for situations where you have to create the object after doing several operations so you can't use the initializer list.


There is no such situation, unless you have very poorly designed and implemented code. Coming up with a design pattern to support and obfuscate the use of poorly designed or implemented code is not something I would condone with any great degreee enthusiasm.

Now, consider this as a better implementation of your example. It compiles and runs.

#include <iostream>

using namespace std;

class A {
public:
A(int) {}
};

struct UserData
{
UserData() {
cout << "Enter data" << endl;
int data;
cin >> data;
}

int data;
};

class B {
public:
B()
: a(UserData().data)
{ }

private:
A a;
};

int main()
{
B b;
}

Share this post


Link to post
Share on other sites
Quote:
Original post by KulSeran
I think the RAII post is more to the point of:


In that example the lock will be released at the end of the outer scope, so A is not locked (but B and C are).

But why did you create the holder in the outer scope and initialize it in the inner scope? Why would you even want to use it in this case? A simple stack allocated object will suffice.

Quote:
Original post by Bregma
Quote:
Original post by Gage64
... this is for situations where you have to create the object after doing several operations so you can't use the initializer list.

There is no such situation, unless you have very poorly designed and implemented code.


Well I can't think of an example off the top of my head, but are you saying there's never a situation where you have to do something like this:


class B {
public:
B(/*whatever*/);
};

class A {
public:
A() {
// Do some things that will "generate" the data needed to initialize B
// ...

// Now initialize B
b = new B(/*the data generated in the previous steps*/);
}

private:
B *b;
};




Quote:
Now, consider this as a better implementation of your example.


Why is that better?

Share this post


Link to post
Share on other sites
Quote:
Original post by Gage64
Well I can't think of an example off the top of my head, but are you saying there's never a situation where you have to do something like this:
*** Source Snippet Removed ***
That is a job for std::auto_ptr - no need for your own handle class.

Share this post


Link to post
Share on other sites
It isn't. You probably should throw an exception if cin fails, since otherwise data is just uninitialized. (And it also writes to a local data.)

However, you could do input in a function that returns the entered value (and throws if input fails.) Probably in all cases you can either get the data to be used for initialization from a function. Or compute it first and provide a suitable constructor.

But here's your original class in somewhat better style as I believe.

Main changes: assignment operator shouldn't freak out if one or both are empty holders, since it is a valid state for this class. destroy is a private method, since the user doesn't really need it (the example shows how you can reset a holder earlier if you are desperate). asserts should be done in the place where it would be an error to continue (that's the casts in getObj) - for this reason code had to be reordered in the destroy method.


#include <cassert>

// Note: Currently supports objects with zero, one or two parameter c'tors
template <class T>
class Holder {
public:
Holder():created(false)
{
}

~Holder()
{
destroy();
}

Holder(const Holder<T> &other): created(false)
{
if (other.created) {
create(other.getObj());
}
}

Holder<T> & operator=(const Holder<T> &other)
{
if (this == &other) {
return *this;
}

if (!other.created) {
destroy();
}
else if (!created) {
create(other.getObj());
}
else {
getObj() = other.getObj();
}
return *this;
}

T* operator->()
{
return &getObj();
}

const T* operator->() const
{
return &getObj();
}

T & operator*()
{
return getObj();
}

const T & operator*() const
{
return getObj();
}

void create()
{
new (obj) T;
created = true;
}

template <class P1>
void create(const P1 &p1)
{
new (obj) T(p1);
created = true;
}

template <class P1, class P2>
void create(const P1 &p1, const P2 &p2)
{
new (obj) T(p1, p2);
created = true;
}

private:
T& getObj()
{
assert(created && "Holder holds no instance");
return *(reinterpret_cast<T*>(obj));
}

const T& getObj() const
{
assert(created && "Holder holds no instance");
return (const_cast<Holder<T>*>(this))->getObj();
}

void destroy()
{
if (created)
{
getObj().~T();
created = false;
}
}

char obj[sizeof(T)];
bool created;
};

#include <iostream>
#include <string>
using namespace std;


struct Student {
string name;
int grade;

Student(const string &name, int grade)
: name(name), grade(grade)
{
cout << "Student::Student() for " << name << endl;
}


~Student() {
cout << "Student::~Student() for " << name << endl;
}


void print() const {
cout << name << ", " << grade << endl;
}
};


int main()
{
Holder<Student> st1;
Holder<Student> st2;

cout << "Hello" << endl;

st1.create("John", 87);
st2.create("Mike", 92);

st1->print();
(*st2).print();
st2 = st1;
st2->print();

st1 = Holder<Student>();
cout << "Goodbye" << endl;
}

Share this post


Link to post
Share on other sites
Quote:
Original post by swiftcoder
Quote:
Original post by Gage64
Well I can't think of an example off the top of my head, but are you saying there's never a situation where you have to do something like this:
*** Source Snippet Removed ***
That is a job for std::auto_ptr - no need for your own handle class.


Yet again, the difference is that my class doesn't use dynamic memory allocation.

Quote:
Original post by visitor
here's your original class in somewhat better style as I believe.


Thanks, I like your changes, though I do think destroy() should be public, because it lets you destroy an object before the end of the scope. With your approach you can only do that by creating a new object.

EDIT: BTW, I think the assert in the const version of getObj() is redundant.

Share this post


Link to post
Share on other sites
Quote:
Original post by Gage64
Quote:
Original post by swiftcoder
Quote:
Original post by Gage64
Well I can't think of an example off the top of my head, but are you saying there's never a situation where you have to do something like this:
*** Source Snippet Removed ***
That is a job for std::auto_ptr - no need for your own handle class.
Yet again, the difference is that my class doesn't use dynamic memory allocation.
How so? Your create function calls new, so you are using dynamic allocation. That you hide the dynamic allocation from the user doesn't make it any better/faster/whatever.

Share this post


Link to post
Share on other sites
It's placement new, which is called to construct the object in a particular place in memory (in this case a char array which is not dynamically allocated).

I don't see myself using this often but perhaps it isn't entirely meaningless. It is a way to postpone the creation of stack objects. It is not hard to postpone creation of objects if you allocate them dynamically, this is a way to do the same for stack instances.

---------

Perhaps you should also provide a constructor of the form


Holder(const T& val)



so that this works


Holder<X> h;
//...
h = X(a, b, c);



and you could get rid of the create method together with its limitation.

This might have the overhead of creating a temporary but then perhaps the compiler might be able to optimize that away.

Share this post


Link to post
Share on other sites
I can see myself using something like this every once in a while. Usually, it's when I want to construct a by-value object with the results of some computation with the construction arguments.

Once, in my very early days of game programming, I was making a pong clone. I had multiple balls, and I wanted to have the color of the balls alternate, based on a static counter of the number of balls. I had some surface abstraction in the ball class by value, and I had to use a ternary expression and a macro that found out whether the number of balls was even. It would have been nice to have some way to delay the construction of that object.

I've come across a few such situations since. I've always gotten around it, but still...

Share this post


Link to post
Share on other sites
I can't see the advantage of this over a nullable pointer or std::auto_ptr. Actually, auto_ptr's wacky assignment semantics seem tailor-made for this (and for very little else).


void main() {
auto_ptr<Student> st1;
auto_ptr<Student> st2;

cout << "Hello" << endl;

st1 = auto_ptr<Student>(new Student("John", 87));
st1 = auto_ptr<Student>(new Student("Mike", 92));

st1.create("John", 87);
st2.create("Mike", 92);

st1->print();
(*st2).print();
st2 = st1; // this one's a bit wonky, but you can't have everything
st2->print();

st1.release();
st1.release(); // Does nothing

// No call to st2.release();

cout << "Goodbye" << endl;
}


Share this post


Link to post
Share on other sites
The big thing that boost::optional doesn't give you is mid-scope construction and destruction. It has operator=, of course, but that entails an extra copy construction and destruction. The authors seemed to intend it primarily for function arguments and return values, rather than mutable member variables.

Share this post


Link to post
Share on other sites
Quote:
Original post by Sneftel
I can't see the advantage of this over a nullable pointer or std::auto_ptr.


Again, the difference is that my class doesn't use dynamic memory allocation.

Quote:
Actually, auto_ptr's wacky assignment semantics seem tailor-made for this


I'm not sure what you mean. auto_ptr does transfer of ownership. My class doesn't do anything like that.

Unless you mean the assignment operator visitor wrote. Yeah, maybe a better idea is to only allow assignment between initialized or non-initialized holders, but not mixed assignment. Something like:


Holder<T> & operator=(const Holder<T> &other) {
if (this == &other) {
return *this;
}

assert(created == other.created);

if (created) {
getObj() = other.getObj();
}

return *this;
}


Or maybe it's better to just disallow assignment, because I think even then this should work:

*st2 = *st1;

Quote:
Original post by visitor
Perhaps you should also provide a constructor of the form

Holder(const T& val)

so that this works

Holder<X> h;
//...
h = X(a, b, c);

and you could get rid of the create method together with its limitation.


Why is that better than the way it currently works? And what limitations are there to the create() method?

EDIT: Maybe you meant that this would automatically support constructors with any number of parameters. I haven't thought of that, maybe that is a good idea.

Share this post


Link to post
Share on other sites
Another way to approach this problem is from Test Driven Design. How do you do automated testing on class B? You'd need to write a harness that let's you drive cin. The interesting stuff you do in B's constructor makes it difficult to test.

Bregma's version is really slick, and tackles this problem because you could potentially replace the UserData implementation in the test. Typically, though, TDD would suggest doing something like this:


struct A
{
A(int);
}

struct B
{
B(int in): a(in){}
private:
A a;
};

//Might also be an IFactory method
B CreateB()
{
cout << "Enter data" << endl;
int data;
cin >> data;
return B(data);
}




Or even more typically, so that all dependancies are injected:


struct A
{
A(int);
}

struct B
{
B(A in): a(in){}
private:
A a;
};

//Might also be an IFactory method
B CreateB()
{
cout << "Enter data" << endl;
int data;
cin >> data;
return B(A(data));
}



Share this post


Link to post
Share on other sites
Quote:
Original post by Gage64
This is a class that enables you to have control over when an object is created and destroyed without using dynamic memory allocation.



int main {
{ // This is legal C++: an anonymous scope.
std::ifstream foo("foo.txt");
do_a_bunch_of_stuff_with(foo);
} // foo.~ifstream() implicitly called here.
// foo is not in scope here.
more_stuff();
}


If you want a data member of a struct that is allowed to "not be there" at certain times, then another way to say that is that you want a member that is "optional"; in these cases, you may use boost::optional, which is presumably not much different from what you have.

(EDIT: boost::optional not designed for that? Paying extra to reset the value doesn't seem like such a big deal...)

Quote:
For example, if you have an object that doesn't have a default constructor, you can't have an instance of it as a class data member.



class no_default_ctor {
int x;
public:
no_default_ctor(int x): x(x) {}
};

class container {
no_default_ctor contained;
public:
container(int y): contained(y) {}
};


Quote:
I'd like to hear your comments about the usefulness and correctness of this class. I'm just posting this out of boredom, so feel free to be as brutal as you like.


Consider carefully what happens if you copy-construct an instance in the non-created state, or re-create an instance that hasn't been destroyed. I also wouldn't bother writing a 2-arg version of create(), unless you want to keep going with that (Boost "unrolls" things up to ten args typically when it does things like this). The users could just rely on the one-arg version by using the copy constructor (e.g. create(T(x, y, z))). As for the const overload of getObj(), I would just copy the non-const version instead of trying to implement it in terms of the non-const version.

Oh, and the interface is a little clumsy, in that you have to use operator* (or ->) to get at the data, when the class is not clearly trying to model a pointer-like thing.

Quote:
Original post by swiftcoder
However, what does you holder class offer which cannot be matched by std::auto_ptr (or std::tr1::shared_ptr, if auto_ptr isn't enough)?


If fixed, instances of Holder should be usable in standard library containers, which is more than can be said for std::auto_ptr. And shared pointers are often overkill (at least marginally so) for the purposes for which they are used. :)

Quote:
Original post by Gage64
Maybe it was a bad example, but like I said, this is for situations where you have to create the object after doing several operations


So, do the operations, and then call the constructor. Pass any necessary information, resulting from those operations, to the constructor.

If this can't be done in a straightforward way (and it nearly always can), then you use the Factory pattern (or something similar; maybe just a simple static create() function for the class).

Quote:
BTW, although it's unrelated, could you please explain why you shouldn't input data inside a constructor?


It's a textbook example of failure to separate responsibilities.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
As for the const overload of getObj(), I would just copy the non-const version instead of trying to implement it in terms of the non-const version.


I've seen this done to avoid duplicating code. Is there anything wrong with it?

Quote:
Quote:
Original post by Gage64
Maybe it was a bad example, but like I said, this is for situations where you have to create the object after doing several operations


So, do the operations, and then call the constructor. Pass any necessary information, resulting from those operations, to the constructor.

If this can't be done in a straightforward way (and it nearly always can), then you use the Factory pattern (or something similar; maybe just a simple static create() function for the class).


How will that work, for example with the code from here (I used a link to avoid copying it again).

As for the factory, won't that use dynamic memory allocation?

Quote:
Quote:
BTW, although it's unrelated, could you please explain why you shouldn't input data inside a constructor?


It's a textbook example of failure to separate responsibilities.


I'm probably missing something obvious, but could you please explain this a bit more? Is it something like: "the object shouldn't know where it's getting the data from, so it should be passed to it via constructor parameters"?

Share this post


Link to post
Share on other sites

This topic is 3286 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this