Calling an object's constructor?

Hello!

I have a class mp_image, which looks something like this:

Code:

---

class mp_image {
    int w, h;
    byte* pixels; //byte is a typedef of uint8_t;
public:
    mp_image()
    {
        w = h = 0;
        pixels = NULL;
    }
    mp_image(int width, int height)
    {
        w = width;
        h = height;
        pixels = new byte[3*w*h];
    }
    ~mp_image()
    {
        if (pixels) delete[] pixels;
    }
    mp_image& DownsampleNTimes(int n, mp_image& dest_image)
    {
        /* Initialize dest_image here with width w/n and height h/n */

        /* more code... */

        return;
    }
}

---

It's a simplification, but anyway, I would like the member function DownsampleNTimes to take an object dest_image as an argument (by reference) to store the downsampled image in. dest_image must be an empty image (no need for calling it's destructor), and the function shall start by initializing the image so that the right amount of memory gets allocated for holding the pixels of the downsampled image. Is it possible to call the constructor (and even the destructor) of an object, or should objects always be refered to by pointers so that the constructors/destructor are called using the new/delete operators?

Well, if you're passing by reference, the parameter will be an object (that is, it will already be constructed). You can't call the constructor on an object and probably shouldn't call the destructor, because that will get called automatically when the object goes out of scope.

Passing a pointer probably isn't a good idea either because then it's not guaranteed to point to an object.

If I understand correctly, you probably don't need that parameter at all, you can just declare and construct the object withinin the function and return it (meaning it would have to be dynamically allocated unless you wanted to store it in a memory address which could be passed as a parameter, or return by value).

You should also take advantage of initializer lists.

If you can, instead write an init method that will initialize the object for you.

It is possible to construct an object given a pointer to the memory that it will point to using placement new. But generally this is only used for generic allocators responsible for managing memory, not for late initialization of a class.

Calling the destructor is possible, but it can only be done in cases where the destructor would not automatically get called, since then the system will try to destroy the object twice. Again using a destroy method or similar is often a better approach.

Quote:

---

Originally Posted by rudyman

You should also take advantage of initializer lists.

---

How do you mean? Isn't initializer lists only for classes with other class objects as members? My class has no member which needs to be constructed.

And King Mir, your suggestion seems like a good idea, thank you.

You can do:

Code:

---

    mp_image(int width, int height)        : w(width), h(height);
    {
        pixels = new byte[3*w*h];
    }

---

You could also default width and height to zero and check if they either of them is zero in the mp_image constructor - then set pixels to NULL or allocate depending on the result. That way, you don't have to have two different constructors.

--
Mats

You might need to implement a copy constructor and overloaded assignment operator anyway (Rule of Three).

Then you could simply do:

Code:

---

dest_image = mp_image(w/n, h/n);

---

to resize the internal array of dest_image.

>> Isn't initializer lists only for classes with other class objects as members?
No, it's for initializing all member variables. When your class has only built-in types as members it has no effect on performance, but you should still use it for consistency and clarity of code.

BTW, why aren't you using a vector here? I can't think of any reason why a dynamic array would be better, and with vector you wouldn't have to worry about the rule of three.

Quote:

---

Originally Posted by anon

You might need to implement a copy constructor and overloaded assignment operator anyway (Rule of Three).

Then you could simply do:

Code:

---

dest_image = mp_image(w/n, h/n);

---

to resize the internal array of dest_image.

---

I guess this would be an ordinary assignment? So I can overload only the assignment operator. But I guess I could define a copy function as well at the same time. By the way, is there any rule when to copy the data of the internal arrays (such as the pixel values in my case), whether the data should be copied into a new memory allcation or only refered to by another pointer? Does it differ between copying and assignment?

It depends on what you're trying to do. If you make a copy, then changes to one will not affect the other. If you copy only the pointer, then changes to one will show up automatically in the other (because they point to the same data). Which is right for your program?

The safest thing to do is to create new memory and copy the data to it (actually the safest thing to do is use vector to automate that task). This is probably more common in C++.

If you just keep a copy of the pointer, then you have to keep track of all the copies so that delete is called on that memory only once. Your code above calls delete in the destructor, so if you copied the pointer only (which is what it does by default) then two different instances of the class would use the same pointer value and both would try to delete the array when they were destructed. That would be very bad.

And using a vector would save you all this trouble.

Write a swap function for an mp_image, construct the downsampled image into a temporary mp_image and swap it with dest_image before returning.

Code:

---

    void swap(mp_image &other)
    {
        std::swap(w, other.w);
        std::swap(h, other.h);
        std::swap(pixels, other.pixels);
    }
    mp_image& DownsampleNTimes(int n, mp_image& dest_image)
    {
        mp_image temp(w/n, h/n); /* Initialize dest_image here with width w/n and height h/n */

        /* more code... */

        dest_image.swap(temp);
        return dest_image;
    }

---

Can't get much better than that!

Using a vector and following the rule of three could also help.

Why are you making DownsampleNTimes a member function?

--
Computer Programming: An Introduction for the Scientifically Inclined

Quote:

---

Originally Posted by Sander

Why are you making DownsampleNTimes a member function?

---

Because the heavy use of the member variables. Do you think it's a better idea making it extern?

However, I got it working directly after I writing a assignment operator, as required in the rule of three. Thanks.

>> Because the heavy use of the member variables.
Are you using the member variables of the current object (this)? Or are you using the member variables of the passed in object dest_image? If it's the latter, then it doesn't need to be a regular member function, and should probably be either a static member function on non-member friend. A better solution might be to leave it a member function but perform the operation on the this object instead of taking a mp_image reference.

>> However, I got it working directly after I writing a assignment operator, as required in the rule of three.
I still think you should use vector.

Quote:

---

Originally Posted by TriKri

Because the heavy use of the member variables. Do you think it's a better idea making it extern?

---

Yes, like Daved said. I would assume that your image class also has some method to get at the pixel data (which you had elided for briefness). A reasonable rule of thumb is that if a function can be external to your class (perhaps a friend function, if need be) then it probably should.

If you follow this rule of thumb, your data types will consolidate faster. This may not sound too important if you're working on a project all by yourself, but in multi-people projects it's very nice if your image.h header file stabilizes early on in the project, and needs infrequent changes.

--
Computer Programming: An Introduction for the Scientifically Inclined

Quote:

---

Originally Posted by Daved

I still think you should use vector.

---

What is the advantages of using vector? I would still have to declare an assignment operator and a copy assignment operator. Or is is there any other advantage? Btw, do vector copy the array data during assignment? The simplest way can imagine is to use byte* instead of vector (constructors taking adress to the pixels still take void*), and just copy the pointer during assignment, then I don't have to wory about the behaviour of vector either. Else the code

Code:

---

dest_image = mp_image(w/n, h/n);

---

would first create an array of pixeldata, then copy the data into a new memory allocation, and loose the reference to the original data when the assignment has finished. All copying of pixeldata shall be done not by assignment, but a separate copy image function.

Quote:

---

Originally Posted by Sander

Yes, like Daved said. I would assume that your image class also has some method to get at the pixel data (which you had elided for briefness). A reasonable rule of thumb is that if a function can be external to your class (perhaps a friend function, if need be) then it probably should.

If you follow this rule of thumb, your data types will consolidate faster. This may not sound too important if you're working on a project all by yourself, but in multi-people projects it's very nice if your image.h header file stabilizes early on in the project, and needs infrequent changes.

---

That make sense, I should consider using that method instead.

A vector is good because all of its contents is copied when you try to assign it to another vector.
If you store pointers in the vector, the pointers are copied.
If you store the actual data in the vector, the actual data is copied.
This will make sure you don't have to manage the memory yourself. No need for operator =. No need for copy constructor. The vector should handle it all.

>> I would still have to declare an assignment operator and a copy assignment operator.
Would you? I don't think so.

The biggest advantage is that it is less error prone. You don't have to write the copy and destruction operations yourself, they are already done (and done right). You don't have to worry about exceptions in your code, because if an exception is thrown the vector cleans up automatically.

A vector works like a dynamic array you create with new[]. It copies its contents when you copy it. Normally this is what you want. If you want the copy to share the same data (so modifying one changes the other) then you can store pointers in the vector.

Quote:

---

Originally Posted by Elysia

If you store pointers in the vector, the pointers are copied.
If you store the actual data in the vector, the actual data is copied.

---

That is what I don't want. I need an assignment operator which copies the pointer and not the data, else the original data will leak out after assignment:

Code:

---

existing_object = object_class(...);

---

The object created by the object_class(...) should be directly assigned to existing_object.

Quote:

---

Originally Posted by Daved

>> I would still have to declare an assignment operator and a copy assignment operator.
Would you? I don't think so.

---

I compiled and tried to run the program before I had declared the assignment operator, but I got a message saying the program had terminated with a result indicating an error, when trying to assign, and I assume changing a pointer to a vector wouldn't make it better. When I later declared the operator it worked. But maybe that error was caused by another reason? It might well be so.

Quote:

---

Originally Posted by Daved

A vector works like a dynamic array you create with new[]. It copies its contents when you copy it. Normally this is what you want. If you want the copy to share the same data (so modifying one changes the other) then you can store pointers in the vector.

---

Now you got to explain why I should keep a vector of pointers instead of pixel data? That seems totally unmotivated to me.

Anyway, it was what I wanted in this case, unless there is some equally simple and flexible solution to the problem I had with using a constructor for an already existing object.

Quote:

---

Originally Posted by TriKri

That is what I don't want. I need an assignment operator which copies the pointer and not the data, else the original data will leak out after assignment:

Code:

---

existing_object = object_class(...);

---

The object created by the object_class(...) should be directly assigned to existing_object.

---

It will not leak; the data will be copied to the new object. It's a deep copy we're talking about here.
However, if you store pointers in the vector, we'd basically have a shallow copy.

Quote:

---

I compiled and tried to run the program before I had declared the assignment operator, but I got a message saying the program had terminated with a result indicating an error, when trying to assign, and I assume changing a pointer to a vector wouldn't make it better. When I later declared the operator it worked. But maybe that error was caused by another reason? It might well be so.

---

Do you understand why?
Because if you create 2 classes, and assign a to b, then both objects would have the same pointer. And when one goes out of scope, it deletes the memory. When the second goes out of scope, it tries to delete the memory, but fails.
So what you needed was an assignment operator to copy the data to the new instance.
If you want to copy data like that, all you need to do is make that pointer a vector instead, and it will save you from allocating, freeing and copying the data.
No assignment operator necessary, nor destructor, nor even copy constructor.

Now, if you still insist on keeping a pointer (it's more efficient, after all), then the solution would be to use a smart pointer. There's one called shared_ptr, located in std::tr1::shared_ptr, or alternatively, if your compiler does not have TR1 support, boost::shared_ptr (you can download boost for free).
The shared pointer will keep a countdown of the "shared" memory so it will only be destroyed once the last pointer to it goes out of scope.

>> I need an assignment operator which copies the pointer and not the data, else the original
>> data will leak out after assignment

Can you explain further what you mean by this, perhaps with code?


>> I assume changing a pointer to a vector wouldn't make it better.

It would make it better. That's the whole point. When you store a pointer in your class, the compiler copies the pointer, not what it points to. This is bad because you end up with two pointers pointing to the same spot and both of them getting deleted (or one getting deleted before the other is done using it).

When you store a vector in your class, the compiler copies the vector, and the vector is already set up to copy the data correctly. You don't have to write the copy functions or destructor because the compiler's version uses the vector and the vector has already done all that same work for you.


>> Now you got to explain why I should keep a vector of pointers instead of pixel data?

You probably shouldn't. That was just a general comment about copying versus sharing data.

Ok, I see your point. I didn't think the destructor would be called when the object went out of scope, hence I thought there would be a leak. So I would probably want to use something like shared pointers (I have thought of similar solutions before), allthough each chunk of pixeldata is ment to be preserved for only one image. If two images refere to the same data, it is only going to be temporary, like in this case.

Then I have another question: How do C++ decide when an object falls out of scope? If an object is created within a function, I suppose it falls out of scope when the function returns. But how about when the object is also returned by the function? If it's returned by reference (using the & indicator), the object will be needed at a later point and hence can't fall out of scope ... so when does it fall out of scope? And what's the practical difference between a function returning a variable of type my_class and a function returning a variable of type my_class&? Does an object returned by a function fall out of scope as fast as it isn't in use any more?

Thanks

If you declared the variable in the function, then it ceases to exist when the function ends. If you want to return a reference to it, that's too bad -- it still ceases to exist.

If you create the variable with new, then it exists until you get rid of it with delete.

All variables on the stack is cleaned up when the } for that level is found.

Code:

---

void foo()
{
    int x;
    {
        int y;
    } // y dies here
} // x dies here

---

Simple. It doesn't matter how many references or pointers or whatever there's to it. When it goes out of scope, it dies. The end.

>> what's the practical difference between a function returning a variable of type my_class and a function returning a variable of type my_class&? <<

One will make a copy (that might be optimized away), the other will return a reference. If the object you're returning goes out of scope, then the reference will be invalid and you will have problems. If the reference is to an object that doesn't go out of scope (like a global or member variable), then you will avoid the copy but also expose that object to being modified.

Quote:

---

Originally Posted by Daved

One will make a copy (that might be optimized away), the other will return a reference. If the object you're returning goes out of scope, then the reference will be invalid and you will have problems. If the reference is to an object that doesn't go out of scope (like a global or member variable), then you will avoid the copy but also expose that object to being modified.

---

So If I want to assign a returned object to another object, I'd better not return it by reference if it's on the stack, got it.

Thanks!

I think I have come up with a feasible solution, inspired of iMalc's suggestion, and the fact that auto_ptr has its internal pointer set to NULL when it's assigned from by another auto_ptr.

When an mp_image assigns from another mp_image, it sets the pointer inside the other object to NULL, making that object incapable of deleting the pixel data when it moves out of scope. What do you think of that idea?

Quote:

---

When an mp_image assigns from another mp_image, it sets the pointer inside the other object to NULL, making that object incapable of deleting the pixel data when it moves out of scope. What do you think of that idea?

---

It may be workable, but then you have the same disadvantages as auto_ptr: your mp_image does not have normal copy semantics, so it cannot be stored in a standard container (and other containers that rely on normal copy semantics).

Why would I want to enable mp_image:s to be stored in standard containers?

Why not?
Maybe you want a vector of mp_images or a map?
But there's so many more containers in the standard, too, of which all would be broken by auto_ptr.

>> What do you think of that idea?
Still dangerous. Trying to roll your own solution is still going to leave you with problems.

What about shared_array?

Quote:

---

Originally Posted by Daved

>> What do you think of that idea?
Still dangerous. Trying to roll your own solution is still going to leave you with problems.

What about shared_array?

---

What I relized about shared_array is that it will always remove the pixel data after all mp_images using the data have been removed. My class has been given a constructor to create an image from already existing pixel data (such as from a SDL_Surface), meaning that not all mp_images do really own the pixel data, but they still have r/w access to it. Which means that some mp_images do not have permission to remove the pixel data when destroyed (for which I have a flag to tell), but would do it anyway if I use shared_array. I have thought of implementing a simple counter instead of a shared_array, though, and let the mp_image destructor decide when to delete the data or not.

>> meaning that not all mp_images do really own the pixel data
If the mp_image doesn't own the data, then don't have it use new or delete at all and let whatever owns it take care of the memory management. All you have to do is make sure all the instances of your class don't try to use the image once you destroy it. Otherwise that makes it really easy, just store a pointer to the data.

It has other constructors as well, which allocates memory for new pixel data. In that case the object does become responsible for removal of the data.

Just a question, what feels the most natural to you; assignment which copies the pointer and not the data (this would be the same as using memset on the two objects), or assignment which copies the data so you actually have two different but identical images?

The first, with shared pointers (std::tr1::shared_ptr or boost::shared_ptr).
But beware if you try to modify the pixel data later in the class, since it's shared.

>> what feels the most natural to you
They are equal, it depends on your situation.

Are there any scenarios in which both copies will be active at the same time? If not, then copying the data is not a good solution.

If there are those scenarios, then the question is if one object modifies the data should that change be reflected in the other object's data, or should the other object's data be unaffected. If the other object should be unaffected, then you need to copy the data. If the other object should be updated, then you need to share the data.

If you can answer those questions, then you'll be closer to what you need. If the answers are different in different situations, then you might want to re-think your design and make it all work the same or come up with different objects for different scenarios.