Class Copier function doesn't want to copy.

[Tset Tsyung]

Hi all,

I am just testing my new knowledge of Shallow vs. Deep copying by writing my own copy function.

However it's producing this error error: invalid conversion from 'int*' to 'int' any ideas?

Also I wish to point out that I am learning C++ from "C++ for Dummies" which is just awful! Therefore any explanations about WHY I'm going wrong would be wonderful - I hate this book but am determined to finish it before I burn it lol.

Many thanks Guys 'n' Girls.

#include <cstdlib>
#include <iostream>
// This definition is for pre-'09 compilers
#define nullptr 0

using namespace std;

// counter used for none declared ID's
int counter = 1000;

// Class Declarations;
class tester
{
    friend void fnOutput(tester& outputtest);
    public:
        // Constructor
        tester(string pName = "No Name", int pInt = counter++)
            : name(pName), ID(pInt)
        {
            cout << "Creating 'tester' object called " << name
                 << " with ID of " << ID << endl;
        }

        // Copy Constructor
        tester(tester& copiedTester)
        {
            cout << "Copying " << copiedTester.name << endl;
            ID = new int;    // This is the line that causes the error!
            ID = copiedTester.ID;
            name = copiedTester.name;
        }

    protected:
        string name;
        int ID;
};

// Prototype Declerations

// Declarations
void fnOutput(tester& outputtest)
{
    cout << "Outputting " << outputtest.name << endl;
}

int main()
{
    tester test1;
    tester test2(test1);

    return 0;
}

[Athar]

new int creates a new int object on the free store and returns a pointer to it.
ID is just a regular int, so you can't assign a pointer to it.
More importantly, ID already exists, so there is no need at all to create a new int at that point. Just delete the line.

Edit: if you want a shallow copy, make ID a pointer (but still get rid of the problematic line).
The parameter "int pInt" will need to be a pointer as well. If the 'p' is any indication, that's probably what you intended anyway.

[rollie]

In college, I learned from the Deitel 'How To Program C++', which I found very useful. Whenever you write '... = new X;', it dynamically obtains memory for that X and calls X's constructor. Rule #1 of the 'new' operator is that you must have a matching 'delete' call to free up the memory allocated in this way. Also, the return value of this expression is not an object of type X, it's a pointer to an object of type X, or in this case, an 'int *'. Your member variable 'ID' is not an 'int *', it's an int. You can remove the 'ID = new int' line entirely. But, because there aren't any pointers in your tester class, you aren't really demonstrating the difference between shallow and deep copying. Try:

1) change ID to be an int *
2) change the copy constructor to take a 'const tester &' instead of a 'tester &' (a good practice)
3) dynamically allocate memory for ID in the default constructor, like you are doing in the copy constructor
4) copy the value in the copy constructor by setting it to *ID = *(copiedTester.ID); (this is your 'deep copy')
5) delete ID in the destructor

There's a couple other small changes you'll need, but you can likely work them out

--Rollie

[Tset Tsyung]

Thanks for your replies Athar and rollie.

First off thanks to Athar - That thought about 'new' hit me as soon as I walked away from the PC, lol. And pInt isn't supposed to be a pointer - I was just confusing myself (what an idiot I be).

Second, rollie thank you for your tips list. What did you mean by Point 3? Also couldn't get point 4 to work...

Many thanks guys, will soon mark as solved! :)

[S6k9GNh0]

Okay... the copy class is incredibly important. It controls whether the operator= function works for you class and how it works if it is allowed. Making it slow and bulky is the absolute last thing you want to do.

I would also like to state that using dynamic allocation implicitly like this is dangerous and easily leads to memory leaks. Because int is a small type, I would suggest not making it a pointer (since it's usually the same size of a pointer or even smaller sometimes).

http://codepad.org/WvkI0saQ

Please note line 21 with the changed parameters from string to const string&. This allows a constant literal string to be passed to the function while also not copying the actually value as a parameter and only taking a reference (pointer) which lowers the amount of weight passed to the copy function. This also allows the user to pass a value without worrying about pointers which is an advantage of references. However, the value itself is still copied to the new class within the function which is required since its a duplicate class.

EDIT: You could also do much better by taking advantage of initializer lists: http://codepad.org/kCPAvCMI

[kempofighter]

I think that the OP is still missing the point. The tester class does a deep copy automatically. There is no need to write a copy constructor at all. If you want to practice with deep copying then you have to make a class where a user defined copy constructor makes sense. in other words you have to have at least one pointer to something as a class attribute.

[Tset Tsyung]

Ah - think I've got it now!

You only need to do a deep copy when (as you say kempfighter) there is "at least pointer" because you need to make a copy of what the pointer is forwarding to as well! Otherwise change the value in one objecy, you change it in the other as well because they are both looking at the same value in memory! Is that correct?

Also computerquip - what did you mean by using "dynamic allocation implicitly" is dangeous? And is that what I was doing? I only ask because, as I stated at the top I'm teaching my seld C++ with a stupid "Dummies" book that has more value as a door stop - therefore my knowledge of theory and terminology is questionable lol.

Again many thanks for all of your help everyone.

[rollie]

computerquip - agree with most of what you said, but note that Tset is doing this project to understand deep vs shallow copy, not to implement a robust class for use in a production environment.

Tset - Assuming you did #1, your member variable ID (maybe change the name to m_pID?) now is not an integer exactly - it is a pointer to an integer. By default however, no integer has been created, so you need to execute ID = new int; in the default constructor, such that your pointer actually points to a valid int object. #4 will fail if you don't do #3, because the copiedTester.ID would not have been allocated - it would be an integer pointer that isn't pointing to any integer!

To illustrate the deep vs shallow differences, in main, try changing the value of *test1.ID - you will notice that the value of *test2.ID does not change. However, if your copy constructor was changed from

*ID = *(copiedTester.ID);
to
ID = copiedTester.ID;

you would be performing a shallow copy, and altering *test1.ID would also affect *test2.ID.

Note, if you do this shallow copy, you may run into some interesting memory management issues :)

--Rollie

[Tset Tsyung]

Rollie...

Think you got those last two mixed around lol.

Anyways that has now been resolved and made clear, thank you to everyone for that! (Really love this site, just hope I can contribute someday).

New prob though...

Member Function:

        void outputname(tester& outputname)
        {
            cout << "The name of tester 2 is now " << outputname.name << endl;
        }

Calling of Member Function:

    tester test2(test1);
    test1.changename("New Name");
    test2.outputname(test2);                   // this one works fine
    tester::outputname(test2);                // this one produces following error 

Error:

error: cannot call member function 'void tester::outputname(tester&)' without object

Why is this? Surely I'm passing the object in the call tester::outputname(test2);

Also I am aware that I can sort this out by merely changing the member function to:

void outputname()
{
    cout << name << endl;
}

and the call to:

test2.outputname();

... Just wanna fill out my knowledge of what's right and wrong.

LOVE YOU GUYS!!!
ahem.. well... what I mean is... thanks.

[spaggy]

You can't call tester::outputname(test2); like that unless void outputname(tester &) is a static member of the test class. As void outputname(tester &) is not static it can only be called from a tester object and not by the tester class. This is why you get that error message.

[Tset Tsyung]

Thank you all,

Think that's sorted - many thanks again.

Cheers spaggy for clearing that last thing up!