Passing objects in the constructor

Started by
10 comments, last by Antheus 15 years, 4 months ago
I have a base class

class Base
{
public:
    Base (Mesh _mesh, Material _material) :
      mesh (_mesh),
      material (_material),
    {}
private:
    Mesh mesh;
    Material material;
}
Now, I have a class that derives from this class

class B : public Base
{
public:
    B () :
      Base (Mesh (arg1, arg2, arg3),
            Material (arg1, arg2, arg3))
      ...
    {}
}
As you can see, I pass arguments to the base constructor by calling constructors for its arguments. However, the mesh class has a default copy operator, and to avoid a double-free in this situation will need to copy all the memory to another place (ie. override this default copy operator), because it has dynamic memory. It will get created in the scope of B, the passed to Base, copied, and then when it returns the original will be deleted, and in this case will suffer a double-free error if I don't override the copy constructor. It contains alot of data, so I don't want to be copying it. Material is in no such case. (It can be copied fine without a double-free error) Rather than just base the mesh arguments to the base class and have it instantiate these objects directly, is there a better way to do this? I could use pointers but I would have to delete the pointers in the deconstructor of Base, which would go wrong if I it was based a non 'newed' pointer, for example a pointer to data on the stack. Thanks! In Java you'd just use that syntax (not that I'm THAT familiar with java) maybe some insight into how it handles its garbage collection in this case? Or does java just create two copies like I said was happening?
Advertisement
You'd pass them as const references (const Mesh&). There will be one copy made (as the object takes the ownership) but passing references ensures that no more copies are made no matter how many times you pass the arguments on.
Quote:Original post by solinent

In Java you'd just use that syntax (not that I'm THAT familiar with java) maybe some insight into how it handles its garbage collection in this case? Or does java just create two copies like I said was happening?


Java has no value semantics, so everything would be a pointer. The following would be functionally equivalent to Java:
class Base {...    shared_ptr<Mesh> mesh;    shared_ptr<Material> material;}class B : public Base {public:    B () :      Base (new shared_ptr<Mesh>(new Mesh (arg1, arg2, arg3)),            new shared_ptr<Material>(new Material (arg1, arg2, arg3))) {} ...}


Quote:However, the mesh class has a default copy operator, and to avoid a double-free in this situation will need to copy all the memory to another place (ie. override this default copy operator), because it has dynamic memory


If mesh dynamically allocates memory, and doesn't follow rule-of-three, you have a bug or faulty implementation. And that's it.

Default versions do not handle dynamic memory.

To avoid extra copy, you can either use above smart pointers, use those inside Mesh class, or modify constructors, so that instead of passing mesh, you pass pointers. Passing by reference may avoid extra copy, but isn't required to. It still needs to create temporary object, and copy from that.

Note that you also perform a redundant copy in Base() constructor, since you're passing Mesh and Material by value.
Quote:Original post by solinent
However, the mesh class has a default copy operator, and to avoid a double-free in this situation will need to copy all the memory to another place (ie. override this default copy operator), because it has dynamic memory. It will get created in the scope of B, the passed to Base, copied, and then when it returns the original will be deleted, and in this case will suffer a double-free error if I don't override the copy constructor. It contains alot of data, so I don't want to be copying it.


There are two possibilities in C++:
  • Your objects can be copied. In that case, their copy constructor (either default or user-provided) performs that copy.
  • Your objects cannot be copied. In that case, their copy constructor is disabled (by making it private and providing no implementation).


There are no ifs or buts or clever alternatives: either they can or they can't.

Objects which can be copied tend to be copied. Objects which cannot be copied tend to be manipulated by reference and are often wrapped with shared ownership smart pointers. In your example, I suspect that meshes are not intended to be modified, so I would make the mesh copyable, and implement it internally as a shared read-only object containing the data (meaning that copying the mesh is a pointer copy, and the shared read-only object which contains all the data is never copied).
I'll look into shared_pointer (I've always tried to avoid dynamic memory, but I only have it in my mesh class).

I'm not super-experienced in C++, so please forgive me if I sound stupid.

Frankly, I think the rule-of-three is stupid, there may be an elegant solution that violates this rule (unforseen by me), and that there should be no "rules" as long as you weigh out the solutions.

Having said that, using a const reference will not work because when using Mesh () in this way:

class B : public Base{public:    B () :      Base (Mesh (arg1, arg2, arg3),            Material (arg1, arg2, arg3))      ...    {}}


Won't the reference be invalid after Mesh's destructor gets called (I don't know where the destructor will be called, I assume it will be called at the end of the constructor?).

My solution (inelegant, but probably less "ugly") would be to alter the Base constructor and pass it arguments to create a Mesh. This allows me to ignore Base-specific properties that need to be initialized in mesh (there aren't any, but if they come up).

class B : public Base{public:    B () :      Base (arg1, arg2, arg3,            Material (arg1, arg2, arg3))      ...    {}}


I'll see what happens when I use a constant reference also.
Quote:Original post by solinent

Frankly, I think the rule-of-three is stupid, there may be an elegant solution that violates this rule (unforseen by me), and that there should be no "rules" as long as you weigh out the solutions.


Rule-of-three is the most elegant solution. It solves the problem of C++ memory model, object life-cycle, implicit conversions and value/reference semantics.

Quote:Won't the reference be invalid after Mesh's destructor gets called (I don't know where the destructor will be called, I assume it will be called at the end of the constructor?).


Absolutely required reading.

Quote:My solution (inelegant, but probably less "ugly") would be to alter the Base constructor and pass it arguments to create a Mesh. This allows me to ignore Base-specific properties that need to be initialized in mesh (there aren't any, but if they come up).


Code can be made to compile in many ways. Compiler tolerates everything.

Regardless of how you construct Base, unless you respect the rule of three with all the details, you have a ticking bomb. See articles above for reasons as to why. The other alternative is to make Mesh or Base non-copyable.

Results will be undefined behavior, memory leaks, or if you get lucky, crashes.
Quote:Original post by Antheus
Quote:Original post by solinent

Frankly, I think the rule-of-three is stupid, there may be an elegant solution that violates this rule (unforseen by me), and that there should be no "rules" as long as you weigh out the solutions.


Rule-of-three is the most elegant solution. It solves the problem of C++ memory model, object life-cycle, implicit conversions and value/reference semantics.

Quote:Won't the reference be invalid after Mesh's destructor gets called (I don't know where the destructor will be called, I assume it will be called at the end of the constructor?).


Absolutely required reading.

Quote:My solution (inelegant, but probably less "ugly") would be to alter the Base constructor and pass it arguments to create a Mesh. This allows me to ignore Base-specific properties that need to be initialized in mesh (there aren't any, but if they come up).


Code can be made to compile in many ways. Compiler tolerates everything.

Regardless of how you construct Base, unless you respect the rule of three with all the details, you have a ticking bomb. See articles above for reasons as to why. The other alternative is to make Mesh or Base non-copyable.

Results will be undefined behavior, memory leaks, or if you get lucky, crashes.


Why will I get undefined behavior? I'm creating a local copy of Mesh by passing it into B. I'm only creating a Mesh once in the local static memory of the base class.

You may have more knowledge of c++ than me (Instead of reading the C++ FAQ, I read the C++ FQA: http://yosefk.com/c++fqa/index.html).

However, I fail to see you suggest an answer or even point out what's wrong with my program. If I've missed it, please forgive me.

Quote:Code can be made to compile in many ways. Compiler tolerates everything.


Compiler tolerates code that will compile. Code that compiles is seen by the compiler as correct. Since coders are human, all code is inherently incorrect. This is the source of all bugs of all time, and you telling me this does not help me, sorry.

In case you're being genuine and really want to help my situation, this is what I'm doing right now:
class Base{public:    Base (XX arg1, XX arg2, XX arg3, Material _material) :      mesh (arg1, arg2, arg3),      material (_material),    {}private:    Mesh mesh;    Material material;}class B : public Base{public:    B () :      Base (arg1, arg2, arg3,            Material (arg1, arg2, arg3))      ...    {}}


As you can see, this solves my problem, but breaks your "rule-of-three" because I repeat the arguments 3 times. However, there are no copies in memory.

I can't create/initialize a static copy of a Mesh (within B) and pass it as a constant reference to Base because Base is initialized before everything and therefore wouldn't have access.

I'd have to pass static copies through Base and this almost certainly isn't a good solution.

I could use a boost::shared_pointer as specified above, but I would rather not include boost in my code (creates more dependencies).

Thank you. I will read the C++ FAQ one more time, just incase it answers my question. Could you reference me a good C++ book that I could use as a reference, also?
Quote:Original post by solinent
Frankly, I think the rule-of-three is stupid, there may be an elegant solution that violates this rule (unforseen by me), and that there should be no "rules" as long as you weigh out the solutions.


Let's just say that if you want to violate the rule, you better have a damn good reason for it.

Quote:using a const reference will not work because when using Mesh () in this way:

class B : public Base{public:    B () :      Base (Mesh (arg1, arg2, arg3),            Material (arg1, arg2, arg3))      ...    {}}


Won't the reference be invalid after Mesh's destructor gets called (I don't know where the destructor will be called, I assume it will be called at the end of the constructor?).


Here you are creating a temporary mesh object. It's destructor will be called when the constructor for Base finishes executing, and by that time the mesh will be copied over to the mesh variable stored in B. The reference won't become invalid because it will "disappear".

It's like saying that, in the following function:

void func() {    int i = 7;    int *ip = &i}]


ip becomes invalid when the function exits, which is not true because when the function exits ip will no longer exist.
Quote:Original post by solinent
Why will I get undefined behavior? I'm creating a local copy of Mesh by passing it into B. I'm only creating a Mesh once in the local static memory of the base class.


Passing arguments for constructing a Mesh, instead of passing a Mesh directly, is not a source of undefined behaviour. What is potentially a source of undefined behaviour, is the behaviour of the copy constructors, destructors and copy assignment operators of the classes involved.

When you follow the rule of three, it is trivial to verify that resources that are allocated all get deallocated exactly once and in due time. With the copy-and-swap idiom, it is also trivial to ensure that this works even in the presence of the possibility of (a) an exception thrown in a constructor and (b) self-assignment.

Quote:However, I fail to see you suggest an answer or even point out what's wrong with my program. If I've missed it, please forgive me.


He's trying to give you general advice. Very little can be pointed out about "what's wrong with your program" because you've shown very little of your program.

Quote:Compiler tolerates code that will compile. Code that compiles is seen by the compiler as correct. Since coders are human, all code is inherently incorrect. This is the source of all bugs of all time, and you telling me this does not help me, sorry.


His whole point was to remind you of these facts, actually. We all know that code is not necessarily "correct" if it compiles. The point is that by following simple guidelines (such as the rule of three), it becomes much easier to write correct code. By disregarding such rules you throw away safety for little to no gain, unless your situation is very specific (and even then it is often not worth the effort). That the compiler can't just take care of these things for you automatically is a consequence of working in a lower-level language such as C. Languages like C# and Java effectively implement these things for you behind the scenes (except that they generally use garbage collection instead of implementing destructors, and avoid copy construction by using reference semantics for objects).

Quote:In case you're being genuine and really want to help my situation, this is what I'm doing right now:

...

As you can see, this solves my problem, but breaks your "rule-of-three" because I repeat the arguments 3 times.


The rule of three has nothing to do with the repetition of arguments. It's this concept.

Quote:I can't create/initialize a static copy of a Mesh (within B) and pass it as a constant reference to Base because Base is initialized before everything and therefore wouldn't have access.


Yes, you can. That is what the initialization list does for you: it allows you to specify the parameters used for the constructors that initialize the bases and members of the class. When you write

B::B(args) : Base(Mesh(args))


, you say "When the B constructor is called with args, the initialization of the Base base shall use its Base::Base(const Mesh&) constructor, passing it a temporary Mesh constructed with Mesh::Mesh(args)".

The order of events is:

Mesh::Mesh(args) constructs an unnamed Mesh.
The Mesh is passed to Base::Base(const Mesh&). The initialization of the Base does whatever with this const Mesh&; likely, it copies the Mesh into a data member, implicitly using Mesh::Mesh(const Mesh&).
Base::Base(const Mesh&) returns.
The temporary Mesh is destructed with Mesh::~Mesh(). There are no longer any references to this Mesh (because they were just parameters to functions that have already exited), so there is no problem with invalid references.
The rest of the B constructor runs.

Quote:I could use a boost::shared_pointer as specified above, but I would rather not include boost in my code (creates more dependencies).


Why do you think dependencies are a problem?
Why not simply create a six-argument constructor for base (or three, if those args are supposed to be the same) and construct the Mesh and Material objects inside the base class where they should be constructed?!
"In order to understand recursion, you must first understand recursion."
My website dedicated to sorting algorithms

This topic is closed to new replies.

Advertisement