Solved

Copy Constructor

Posted on 2001-07-24
13
257 Views
Last Modified: 2010-04-02
I'm trying to add a copy constructor and operator= to a class, but I keep running into problems. I wrote them like this:

class Yada
{
private:
    int data;

public:
    Yada( const Yada &copy );
    Yada& operator=( Yada &copy );

    int GetData()  { return data; }
};

Yada& Yada::operator=( Yada &copy )
{
    if (this == &copy)
        return *this;

    data = copy.GetData();

    return *this;
}

Yada::Yada( const Yada &copy )
{
    *this = copy;
}


When I compiled them, the line "*this = copy;" caused an error because "const Yada &copy" couldn't be converted to the "Yada &copy" that operater= expected. I tried changing it to the following, thinking that a cast would fix things:

Yada::Yada( const Yada &copy)
{
    *this = (Yada)copy;
}

This turned out to be a Very Bad Thing, as Yada's copy constructor became recursive and overflowed the stack. :-)
Giving up on that approach, I tried copying the code from operator= into the copy constructor.

Yada::Yada( const Yada &copy )
{
    data = copy.GetData();
}

When compiled, it produced an error that said, "'GetData' : cannot convert 'this' pointer from 'const class Yada' to 'class Yada &'." This confused me; I don't understand why it's illegal to call a const class's member functions.

Anyway, if that problem can't be resolved, then I think that I need two operator= functions - one const, and the other not const. However, I've never seen anyone else use two operator= functions in this way. Am I missing something here? Is there a quick fix that I don't see, or have I already stumbled onto the solution on my own?
0
Comment
Question by:TookH
  • 6
  • 3
  • 2
  • +1
13 Comments
 
LVL 30

Expert Comment

by:Axter
ID: 6315834
Your operator= should be using a constant

Yada& Yada::operator=(const Yada &copy )
0
 
LVL 30

Expert Comment

by:Axter
ID: 6315840
Although it's not required, it is good practice to also have a const in the return of the operator=

const Yada& Yada::operator=(const Yada &copy)
0
 
LVL 30

Expert Comment

by:Axter
ID: 6315844
>>then I think that I need two operator= functions

No, you just need the one with the const.

Is there a reason why you don't want to put const in the operator?
0
Problems using Powershell and Active Directory?

Managing Active Directory does not always have to be complicated.  If you are spending more time trying instead of doing, then it's time to look at something else. For nearly 20 years, AD admins around the world have used one tool for day-to-day AD management: Hyena. Discover why

 

Accepted Solution

by:
leorz earned 50 total points
ID: 6315849
for copy c'tor:

Yada::Yada( const Yada &copy )   // signature OK
  {
        //   *this = copy;                 // nah, assigment is not construction, even though in this trivial
                                                    // class you could probably make it work if you really tried. But don't bother...
       data = copy.data;              //... just grab data from the source object
  }

Of course, best thing is omit the copy c'tor completely. The generated one will work just fine!


for assignment operator:

Yada& Yada::operator=( const Yada &copy )   // make it a const ref, the canonical copy assignment op.
  {
       if (this == &copy)
              return *this;

     //  data = copy.GetData();     // why bother with the accessor?
      data = copy.data;                  // privacy is for the *class*, not the *object*

        return *this;
  }

But again, the generated assignment op for this class is just fine, so you don't really
need to write one.




0
 
LVL 1

Author Comment

by:TookH
ID: 6316256
Axter:

>Your operator= should be using a constant
>Is there a reason why you don't want to put const in the
operator?

If I make it const, GetData() won't work.

>Although it's not required, it is good practice to also
>have a const in the return of the operator=

Wouldn't that make expressions like "(yada1 = yada2) = yada3;" illegal? What would the benifit of that be?


leorz:

>Of course, best thing is omit the copy c'tor completely.
>The generated one will work just fine!

Yada is just an example class. The real class that I'm implementing this in has a vector of pointers to allocated data that must be duplicated in the copy.

>privacy is for the *class*, not the *object*
>... just grab data from the source object

Oh. :-)
Is that how it's normally done?
0
 
LVL 9

Expert Comment

by:jasonclarke
ID: 6316825
> Is that how it's normally done?

yes, you don't have to worry about access to the other class, it is the *same* class after all.

But, the problem is purely with const, you could use the accessor functions.  You can only call (withouth casting) const member functions on const objects (this means that you adhere to the const guaranty - you cannot pass a const object to a method that might change the logical state of the object).

To make your accessor const, you could just code it like this:

int GetData() const { return data; }

then your copy constructor should work with a const argument.

However, as stated earlier, it is generally not good practise to just call the assignment operator in the copy constructor - they are often quite different.

> What would the benifit of that be?

because expressions like that can be somewhat confusing.  Not everybody agrees that this should be prohibited, however, if you return a non-const reference your type will behave like a built-in.

BTW, some current thinking says that the check for self-assignment can be bad (because it implies that in the presence of exceptions your code is not safe).  

The 'exception safe way' is to do the copy 'off to the side', before making an changing any values, so with a vector that had contents that needed copying, you would do it something like this:

class X
{
private:
  std::vector<Y*> yvec;
public:
  X& operator=(const X& rhs)
  {
    // Stuff that might fail...
    std::vector<Y*> temp;
    for (size_t i=0; i<rhs.yvec.size(); i++)
    {
       Y* newY = new Y(*rhs.yvec[i]);
       temp.push_back(newY);
    }

    // Now do the assignment...
    yvec.swap(temp);

    // Probably need to clear out the old stuff...
    for (size_t j=0; j<temp.size(); j++)
    {
        delete temp[j];
    }

    return *this;
  }
};
0
 
LVL 9

Expert Comment

by:jasonclarke
ID: 6316834
> because expressions like that can be somewhat confusing

I should have said, also because of the exception issue again.  If an exception is thrown within some part of a chained assignment, you have no easy way to tell which parts of the assignment failed and which succeeded, which could be bad.
0
 
LVL 30

Expert Comment

by:Axter
ID: 6316903
Hi (leorz), welcome to EE.
All of the experts here, for the most part have learn from other experts as to the proper etiquette for posting answer.

An answer should not be posted as an answer, if other experts have previously posted possible answers as comments, and/or have already made contributions to the question.

There are many experts who never post answers as answer.  Instead, they post their answers as comments.
If you read the following link, you'll see why this is the preferred method for many of our valued experts, including myself.

http://www.experts-exchange.com/jsp/cmtyQuestAnswer.jsp


Hi (TookH):
Feel free to click the [Reject Answer] button near (Answer-poster's) response, even if it seems like a good answer.
Doing so will increase your chance of obtaining additional input from other experts.  Later, you can click the [Select Comment as Answer] button on any response.
0
 
LVL 30

Expert Comment

by:Axter
ID: 6316919
>>What would the benifit of that be?
It makes your code safer.
I'll post you an example later today.
0
 

Expert Comment

by:leorz
ID: 6317049
Axter:
When I began composing my anwer, there were no other answers *or* comments on the
question. But sorry for the perceived lack of etiquette!
  -leor
0
 
LVL 9

Expert Comment

by:jasonclarke
ID: 6317328
> When I began composing my anwer,
> there were no other answers *or* comments

if it's any help, not everybody agrees with Axter's point of view on this.  Some of us think that if you believe that what you are posting is a reasonable answer, then you are entitled to use the answer button.

The only real rules are those described in the site guidelines and common ettiquette.  There are no special rules for this group (the C++ area).  

But I really don't want to get into another argument about this, so I don't really want to say anything else on the subject...
0
 
LVL 30

Expert Comment

by:Axter
ID: 6317412
>>But sorry for the perceived lack of etiquette!
No problem.
As jasonclarke said, there is no hard-core rule about this other then what is on the link I posted.  However many experts post answers as comments, but clearly not all.

The comment I posted was not meant to be derogatory or negative in anyway.  In fact it's a template comment that I use frequently and all I do is copy and paste it, and change the names.
Just take it as friendly info.
I look forward to working with you in helping questioners in the future.
0
 
LVL 1

Author Comment

by:TookH
ID: 6343276
It works now. Thanks everyone.
0

Featured Post

Netscaler Common Configuration How To guides

If you use NetScaler you will want to see these guides. The NetScaler How To Guides show administrators how to get NetScaler up and configured by providing instructions for common scenarios and some not so common ones.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

Errors will happen. It is a fact of life for the programmer. How and when errors are detected have a great impact on quality and cost of a product. It is better to detect errors at compile time, when possible and practical. Errors that make their wa…
This article shows you how to optimize memory allocations in C++ using placement new. Applicable especially to usecases dealing with creation of large number of objects. A brief on problem: Lets take example problem for simplicity: - I have a G…
The goal of the video will be to teach the user the difference and consequence of passing data by value vs passing data by reference in C++. An example of passing data by value as well as an example of passing data by reference will be be given. Bot…
The viewer will be introduced to the member functions push_back and pop_back of the vector class. The video will teach the difference between the two as well as how to use each one along with its functionality.

773 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question