none
operator overloading and copy constructor

    Question

  • Hi everybody

    Please have a look at the program below. It runs successfully without a user-defined copy constructor. But with the user defined constructor, it crashes at the run-time. What have I got wring?

    Regards

    Chong++

    +++++++++++++++++++++++++++++++++++++++++++++++++++++++++

    #include <iostream.h>

    class CAT {
     int *itsAge;
    public:
     CAT(){ itsAge = new int; *itsAge=5;}
     int GetAge() const {return *itsAge;}
     void SetAge(int age){ *itsAge = age; }
     CAT(const CAT &rhs){cout<<"Copy constructor\n";}//This causes a run-time crash
     ~CAT(){}
     CAT operator=(const CAT &r) {
      if (this == &r) {cout << "Identical\n";
                       *itsAge = r.GetAge() + 1;return *this;}

      //delete itsAge;
      //itsAge = new int;
      *itsAge = r.GetAge() + 1;
      
      return *this;//A copy of the implicit argument is created? It calls a copy
                   //constructor, if there is any?
     }
     };

    main()
    {
     CAT frisky;

     cout << "frisky's age: " << frisky.GetAge() << "\n";

     cout << "Setting frisky to 6\n";
     frisky.SetAge(6);

     CAT whisker;

     cout << "whsiker's age: " << whisker.GetAge() << "\n";

     cout << "copying frisky to whisker\n";
     cout << "whisker's age: " << (whisker = frisky).GetAge() << '\n';

     cout << "whisker's age :" << whisker.GetAge() << "\n";
     cout << "frisky's age :" << frisky.GetAge() << "\n";

     cout << (whisker=whisker).GetAge() << '\n';
    }//main

    // When run without a user-defined copy constructor, this program prints the following:
    //  frisky's age :5
    //  Setting frisky to 6
    //  whisker's age :5
    //  copying frisky to whisker
    //  whisker's age :7
    //  whisker's age :7
    //  frisky's age :6
    //      Identical
    //      8


    Friday, March 01, 2013 4:58 PM

Answers

  • First, your copy assignment operator really wants to return a REFERENCE to the type, not a copy.

    That coupled with the fact that your copy constructor is bogus cause the crash.  Since you define the assignment operator to return a copy, it calls the copy constructor.   Your copy constructor leaves itsAge uninitialized and hence any further attempt to use it is bogus.   Similarly, your destructor is bogus.

    Presuming there's some good reason why you're allocating pointers for no good reason (why not just use an int)

        CAT& operator=(const CAT& r) {
              ...
             return *this;
        }
    

    Friday, March 01, 2013 6:19 PM
  • with the user-defined copy constructor

    CAT(const CAT &rhs){itsAge=new int;*itsAge=rhs.GetAge();copy<<"copy instructor\n";}

    The program did not crash for the obvious reason.

    Thanks.

    Chong


    Friday, March 01, 2013 7:37 PM

All replies

  • First, your copy assignment operator really wants to return a REFERENCE to the type, not a copy.

    That coupled with the fact that your copy constructor is bogus cause the crash.  Since you define the assignment operator to return a copy, it calls the copy constructor.   Your copy constructor leaves itsAge uninitialized and hence any further attempt to use it is bogus.   Similarly, your destructor is bogus.

    Presuming there's some good reason why you're allocating pointers for no good reason (why not just use an int)

        CAT& operator=(const CAT& r) {
              ...
             return *this;
        }
    

    Friday, March 01, 2013 6:19 PM
  • with the user-defined copy constructor

    CAT(const CAT &rhs){itsAge=new int;*itsAge=rhs.GetAge();copy<<"copy instructor\n";}

    The program did not crash for the obvious reason.

    Thanks.

    Chong


    Friday, March 01, 2013 7:37 PM
  • Yes, but you really don't want to return an rvalue from the assignment operator.    The standard assignment operator always returns a REFERENCE to the the current object.

    Also, you should implement the destructor fully.   There's a thing called the rule of 3 in C++:

    If you need to define a user-defined copy constructor, copy assignment operator, or destructor, you most likely need all three.

    You still haven't explained why you are messing with pointers and introducing bugs all over the place rather than using int.    It's always preferable to use data members with their own reasonable copy semantics, actual objects or vector or smart pointers than having to worry about doing things on your own.

    Friday, March 01, 2013 7:59 PM
  • Hi Ron

    I am trying to understand C++, not the standard ways to write good C++ programs. The program here is a modification of an exercise program in a text  book.  I am just streching my undetstanding of C++. By the way, I forgot to mention "delete itsAge;" in the destructor.

    Regards

    Chong

    Monday, March 04, 2013 2:41 PM