Create a copy from "this" pointer and changing the copy without affecting "this" C++

Question

I have this function and I can't modify it

Image z("abc.jpg");
Image g = z.change();

in function Change, I need to return a new value in g without affecting z.

My main problem is in the next function

Image Image::change(){
    Image temp = *this;
    ...
    temp.image[i][j] = mean; // here also this->image[i][j] changed to the same value
}

in this function whenever I change temp , this changes and this is not what I want

the copy constructor

Image::Image(const Image &obj):imageHeader("Temp")
{
    width = obj.width;
    height = obj.height;
    image = obj.image;
    imageHeader = obj.imageHeader;
}

We need to see the full code of `Image`, and possibly of resources it owns, to help you with this problem. — Jeffrey Bosboom, Dec 22 '14 at 04:30
Write the copy constructor correctly. Without seeing it, I can't tell you how to fix it. — Benjamin Lindley, Dec 22 '14 at 04:31
You have written your `Image` class (maybe inadvertently) so that instances of it shares resources with copies made of that instance. — M.M, Dec 22 '14 at 04:32
Given that we don't know how `Image` class is actually implemented, it's very hard to say how to fix this. But my guess is that you have't followed "rule of three", and you're allocating memory in the constructor, but then directly copying the pointers/references to that in your copy constructor (or haven't got a copy constructor) — Mats Petersson, Dec 22 '14 at 04:33
@AhmedShaher - What you posted is not a copy constructor. A copy constructor takes a reference, not a pointer as an argument. — PaulMcKenzie, Dec 22 '14 at 04:37
@AhmedShaher - You could never have fixed it, since a reference uses `.` syntax, not `->` syntax. — PaulMcKenzie, Dec 22 '14 at 04:45
@AhmedShaher - To really answer your question, we need to see what needs to be copied. You never showed us what `image` and `imageheader` are or even what types `width` and `height` are. — PaulMcKenzie, Dec 22 '14 at 04:52

Adam27X · Accepted Answer · 2014-12-22T04:40:04.393

0

What you want is a deep copy. You need to implement a copy constructor for the class Image.

EDIT: The copy constructor that you provided only copies a pointer to the image data instead of copying the data itself (among other issues). This is why you see this behavior.

A signature for such a function would look as follows:

Image(const Image &img) 
{ 
    //copy static members normally and manage data to allocate memory for dynamic members
    width = img.width;
    height = img.height;
    imageHeader = img.imgHeader;
    image = //do something to dynamically allocate the image data here
}

Note also that the rule of three states that you'll probably need a destructor and assignment operator as well.

edited Dec 22 '14 at 04:40

answered Dec 22 '14 at 04:33

Adam27X

889
1
7
16

It's better to have the class only contain member variables with value semantics, so that none of those three functions are required. Look up "Rule of Zero". The rule of three is so last century :) – M.M Dec 22 '14 at 04:37
@MattMcNabb I guess you're saying that the image should really wrapped in some sort of handle class? I'd agree, but chances are that OP is stuck using some old library that doesn't offer that functionality. – Adam27X Dec 22 '14 at 04:38
Can you please explain more what do you mean with "dynamically allocate a new image " – Ahmed Shaher Dec 22 '14 at 04:39
Whatever code is required to add that functionality, put it in its own minimal class and make that be the member variable – M.M Dec 22 '14 at 04:39
@AhmedShaher you have not provided any information so far about what `image` or `imageHeader` are or how they came to be; those are important details – M.M Dec 22 '14 at 04:40
@AhmedShaher You'll likely need to use the new[] operator to dynamically allocate memory. You'll also have to be sure to delete this memory once you're done with it. If `image` is a class it could be as simple as saying `image = new img_data(...params...)` – Adam27X Dec 22 '14 at 04:42
1

@Adam27X: No one needs to use `new[]` to allocate memory, except the library developers who maintain `std::vector` and `std::make_unique`. – Ben Voigt Dec 22 '14 at 04:46
@Adam27X apparently i lake main concepts in oop. Can you help with some resources ? – Ahmed Shaher Dec 22 '14 at 04:51
@AhmedShaher It's not so much OOP as it is the difference between pointers, references as well as between shallow and deep copies. One book I would recommend is Accelerated C++, but this topic has been covered well on SO: http://stackoverflow.com/questions/184710/what-is-the-difference-between-a-deep-copy-and-a-shallow-copy – Adam27X Dec 22 '14 at 04:57

Andrew Lazarus · Answer 2 · 2014-12-22T04:46:51.197

0

What you have "should" work, but only if you have a useful copy constructor. We really need to see what is going on inside Image, but my guess is it looks something like

class Image {
    void* bytesOfPixelData; // maybe a char* or some sort of char[][]
}

When you set Image temp = *this, it just copies your pointer. And since the pointer is shared, then the changes are shared. Classes with pointers in them need more intelligent copy constructors when you want to do a deep copy. Just copying the pointer does a shallow copy. In a case like this, your copy constructor needs to allocate another data buffer and copy the data. The details of that depend on your implementation, which isn't posted.

See also The Rule of Three (or Rule of Five in C++11), for correct design of Image.

edited Dec 22 '14 at 04:46

answered Dec 22 '14 at 04:34

Andrew Lazarus

18,205
3
35
53

I added the copy constructor – Ahmed Shaher Dec 22 '14 at 04:38
@AhmedShaher If you're talking abut the function you added to your original post, that is *not* a copy constructor. – PaulMcKenzie Dec 22 '14 at 04:40
1

Better yet, apply "The Rule of Zero" and use `std::vector` to manage the buffer. – Ben Voigt Dec 22 '14 at 04:44
@BenVoigt Excellent point, although it is possible this is an exercise in learning about copy ctors. – Andrew Lazarus Dec 22 '14 at 04:46

Create a copy from "this" pointer and changing the copy without affecting "this" C++

2 Answers2