Still celebrating National IT Professionals Day with 3 months of free Premium Membership. Use Code ITDAY17

x
?
Solved

C++, copy constructor

Posted on 2013-01-21
5
Medium Priority
?
497 Views
Last Modified: 2013-01-21
Dear experts, please have a look at the code below:

#include <iostream>

using namespace std;

class A{
  private:
            int valueInt;
      public:
            void print();
            void printObject(A objectA);
            void setValue(int value);

            A (A& arg){
                  cout<<"copying constructor: "<<arg.valueInt<<endl;
                  arg.valueInt = 1;
            }

            A (int x);

};

void A:: print(){
      cout<<this->valueInt<<endl;
}

A::A(int x){
      this->valueInt = x;
}

void A::printObject(A objectA){
      objectA.print();

}

void A::setValue(int value){
      this->valueInt = value;
}

int main()
{
      A c(1);
      A d(2);

      c = d; //I would expect copying constructor being called here but it is not. Why?

      c.printObject(d);    //it calls copying constructor which is fine.
//what I do not understand is: why does it print random values?


      return 0;

}

Thank you

panJames
0
Comment
Question by:panJames
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 2
  • 2
5 Comments
 
LVL 40

Accepted Solution

by:
evilrix earned 1920 total points
ID: 38801319
>> //I would expect copying constructor being called here but it is not. Why?
Since c is already constructed it is not calling a constructor... it is performing an assignment. This being the case your copy-constructor is not being executed.

>> /what I do not understand is: why does it print random values?
Well, your copy constructor would appear to be wrong. You should be assigning to "this" not to "arg".

arg.valueInt = 1;

should be

this->valueInt = arg.valueInt;

Also, arg should be passed as a const reference.

Fix those and see what happens. If you still have a problem post the updated code and I'll take a closer look.
0
 
LVL 31

Assisted Solution

by:farzanj
farzanj earned 80 total points
ID: 38801331
It appears to me that in your copy constructor, you are simply modifying a temporary object.  You were supposed to actually copy the value.
Like

valueInt = arg.valueInt
0
 

Author Comment

by:panJames
ID: 38801420
"You should be assigning to "this" not to "arg".

My way of thinking:

A (A& arg){
  cout<<"copying constructor: "<<arg.valueInt<<endl;
  arg.valueInt = 1;

}

arg is passed by reference here so according to my understanding 'arg.valueInt = 1;' should      have assigned this value to arg which in our case is object 'd' and also argument for
"void A::printObject(A objectA){". Where do I get it wrong?
0
 

Author Comment

by:panJames
ID: 38801432
Ok, I got it now!
0
 
LVL 40

Expert Comment

by:evilrix
ID: 38801509
>> Where do I get it wrong?
I'm sure you've figured it out but basically you are performing your initialisation in the wrong direction... you should be initialising "this" with the values from arg and not the other way around.

Also, not really important to this specific question but, you really should use constructor initialisation lists rather than in-constructor assignment. The former (unfortunately) has a slightly uglier syntax but it is the correct way to construct a class. Although the latter works it is very inefficient because you are constructing class members and then assigning to them. You are doing two jobs where only one is required. There are also times (const members, for example) where in-class assignment just won't work (you won't be able to compile the code).

In your case this is what your copy-constructor should look like...

A (A const & arg) : valueInt(arg.valueInt) {
                  cout<<"copying constructor: "<<arg.valueInt<<endl;
            }

http://www.learncpp.com/cpp-tutorial/101-constructor-initialization-lists/
http://www.cprogramming.com/tutorial/initialization-lists-c++.html

Note, how this also makes it clearer which way round the initialisation is performed?

Finally, since your class doesn't contain any reference or pointers and all its members are, in fact, copyable, you don't really need a copy-constructor - the one synthersized by the compiler would work just fine (which performs a shallow rather than deep copy). If this is just an exercise to learn about copy-constructors then do ignore this point, but just bare it in mind.
0

Featured Post

On Demand Webinar - Networking for the Cloud Era

This webinar discusses:
-Common barriers companies experience when moving to the cloud
-How SD-WAN changes the way we look at networks
-Best practices customers should employ moving forward with cloud migration
-What happens behind the scenes of SteelConnect’s one-click button

Question has a verified solution.

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

Unlike C#, C++ doesn't have native support for sealing classes (so they cannot be sub-classed). At the cost of a virtual base class pointer it is possible to implement a pseudo sealing mechanism The trick is to virtually inherit from a base class…
Written by John Humphreys C++ Threading and the POSIX Library This article will cover the basic information that you need to know in order to make use of the POSIX threading library available for C and C++ on UNIX and most Linux systems.   [s…
The viewer will learn how to pass data into a function in C++. This is one step further in using functions. Instead of only printing text onto the console, the function will be able to perform calculations with argumentents given by the user.
The viewer will learn how to user default arguments when defining functions. This method of defining functions will be contrasted with the non-default-argument of defining functions.

722 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