-1

I was told in this answer to delete the member myValue in the destructor. But if I add it I am getting an exception. From the console messages it seems to be entering the destructor at two moments with the same object(!?)

Note: The code is my attempt to an exercise. In it we are given a code to which we can only add to it. Essentially the content of the main and the constructor without parameters is that is given. The rest is what I am adding to it to make it work.

My guess is that a copy of the object made during some function call is destroying the same myValue after exiting the function call. How is the right way to preserve the data pointed to by two objects in this case?

// ConsoleApplication1.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include <iostream>
using namespace std;

class MyFancyInt
{
public:
    // A constructor that inputs a value for initialization, so that line /*1*/ makes sense.
    MyFancyInt(int x) : myValue(new int(x))// The part after the : initializes myValue to point to the newly created int with value x.
    {
        cout << "Constructor that initializes with myValue = " << x << endl;
    };

    //Destructor.
    ~MyFancyInt()
    {  
        cout << "A MyFancyInt destroyed that had the myValue = "<< this->myValue<< " pointing to a "<< *(this->myValue) << endl;
        //delete myValue;
    };

    //A 'Copy Constructor', so that line /*3*/ makes sense. Aparently C++ creates default copy constructors.
    //A copy constructor input an already existing MyFancyInt, creates a new MyFancyInt and initializes it with the data of the former.
    MyFancyInt(MyFancyInt& x)
    {
        myValue = x.myValue;
    };

    //The construtor without parameters already given in the exercise.
    MyFancyInt()
    {
        cout << "Default constructor" << endl;
        myValue = 0;
    }

    // Friend function to overload the operator +. This one takes two MyFancyInt. It is not needed. It can be commented out.
    friend MyFancyInt operator+(MyFancyInt &x, MyFancyInt &y);

    // Friend function to overload the operator +. This one takes an int and a MyFancyInt. It is needed for line /*6*/.
    friend MyFancyInt operator+(int x, MyFancyInt &y);

    // Friend function to overload the operator +. This one takes a MyFancyInt and an int. It is needed for line /*5*/.
    friend MyFancyInt operator+(MyFancyInt &x, int y);

    // Friend function to overload the output stream operator <<. It is needed for the printing in line /*7*/.
    friend ostream& operator<<(ostream& os, const MyFancyInt& x);

    // Overloading the operator =. It is needed for line /*2*/, /*5*/, and /*6*/.
    MyFancyInt& operator=(const MyFancyInt &x);
private:
    int* myValue;

};

MyFancyInt operator+(MyFancyInt &x, MyFancyInt &y)
{
    cout << "MyFancyInt + MyFancyInt = " << x.myValue << " + " << y.myValue << "= " << *(x.myValue) << " + " << *(y.myValue) << endl;
    MyFancyInt z=MyFancyInt(*(x.myValue) + *(y.myValue));
    cout << "= " << *(z.myValue) << endl;
    return z;
};
 MyFancyInt operator+(int x, MyFancyInt &y)
 {
     cout << "int + MyFancyInt = " << x << " + " << y.myValue << " = " << x << " + " << *(y.myValue)<< endl;
     MyFancyInt z = MyFancyInt(x + *(y.myValue));
     cout << " = " << *(z.myValue) << endl;
     return z;
 };
 MyFancyInt operator+(MyFancyInt &x, int y)
 {
     cout << "MyFancyInt + int = " << x.myValue << " + " << y << " = " << *(x.myValue) << " + " << y << endl;
     MyFancyInt z = MyFancyInt(*(x.myValue) + y);
     cout << " = " << *(z.myValue) << endl;
     return z;
 };

 ostream& operator<<(ostream& os, const MyFancyInt& x)
 {
     os << *(x.myValue);
     return os;
 }

 MyFancyInt& MyFancyInt::operator=(const MyFancyInt &x)
 {
     cout << "Entering the assigment operator." << endl;
     myValue = x.myValue;
     return *this;
 };

int _tmain(int argc, _TCHAR* argv[])
{
    MyFancyInt mfi1(1);     /*1*/
    MyFancyInt mfi2 = mfi1; /*2*/
    MyFancyInt mfi3(mfi1);  /*3*/
    MyFancyInt mfi4;        /*4*/
    mfi4 = mfi3 + 2;        /*5*/
    mfi4 = 3 + mfi3;        /*6*/
    cout << mfi4 << endl;   /*7*/
    return 0;
}
Community
  • 1
  • 1
Kae
  • 279
  • 2
  • 10
  • Your copy constructor (and `operator+`) has no reason to take a modifiable `x`. It also does what the implicit copy constructor does, which is wrong for how you use the class. Also, none of your functions need semicolons after their definitions. – chris Jul 22 '14 at 16:47
  • What console messages? – ecatmur Jul 22 '14 at 16:47
  • `int * p = new int, * q = p; delete p; delete q;` – Kerrek SB Jul 22 '14 at 16:47
  • Note that line "2" invokes the copy-constructor and not the copy-assignment operator. Same with "3". The copy-assignment operator gets invoked on "5" and "6". – Some programmer dude Jul 22 '14 at 16:48
  • @chris This is part of an exercise (let me add it to the question). In the exercise there is code you are given and you can only add to it. The copy constructor is that way to make the given line /*1*/ work well. – Kae Jul 22 '14 at 16:49
  • first off, you don't need a pointer for this. Second, you need to allocate and copy the value of `x.myValue`, otherwise you would have a pointer to the same `int` (which would not be a problem without the pointers). – crashmstr Jul 22 '14 at 16:49
  • @Karene, #1 does not need a copy constructor. It's direct-initialization. – chris Jul 22 '14 at 16:49
  • @Karene - Please read this: http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three After you read it, please fix your copy constructor and assignment operator. – PaulMcKenzie Jul 22 '14 at 16:53
  • @PaulMcKenzie So essentially, to avoid the 'dangling pointers' the copying of objects having a member that is a pointer can't be a proper copy? So, if A is a MyFancyInt and B is going to be a copy I need to create a new int, copy the *(A.myValue) to it and point the B.myValue to it? Isn't there a way in which the destruction is handled better in order to make the copying be a strictly speaking copy, i.e. A.myValue exactly equal to B.myValue as addresses? – Kae Jul 22 '14 at 17:01
  • @Karene, You can do that (it's the default copy constructor behaviour) as long as your destructor ensures the memory isn't freed more than once, but then any changes made to one affects the other. Not to mention one object being destroyed would leave the rest with dangling pointers. – chris Jul 22 '14 at 17:05
  • @chris I guess that is expected. But during destruction ... maybe the destruction of an already destroyed pointer shouldn't(!?) be a problem(!?) – Kae Jul 22 '14 at 17:08
  • @Karene, Freeing the same memory twice is undefined behaviour. That's what null pointers are good for. – chris Jul 22 '14 at 17:09
  • @chris Is there a way to solve my question in that direction? I mean, how to do the "as long as your destructor ensures the memory isn't freed more than once"? – Kae Jul 22 '14 at 17:10
  • @chris Oh! I guess I could free the memory and point the pointer to null. Is that it? – Kae Jul 22 '14 at 17:10
  • @Karene, You could, but it still has aforementioned problems. – chris Jul 22 '14 at 17:11
  • @Karene - What you're looking for is a `smart pointer` that has a reference count, such as `std::shared_ptr`. – PaulMcKenzie Jul 22 '14 at 21:54
  • @PaulMcKenzie That is good. I will take it into account for my own programs. For this one I can't use it. The definition of the pointer myValue is in the part of the code this exercise doesn't allow me to change. – Kae Jul 23 '14 at 02:53
  • @Karene - The answer of `shared pointer` is the hint as to what you're trying to achieve. You must ensure that `delete` is called only once, and the classical way to do that is keep track of the number of objects that are sharing that int, and only call `delete` when you detect that no one is sharing that int anymore (the last object is destroyed). How you do that? That is an exercise you have to undertake, but the hint is to look at how a shared pointer is implemented. – PaulMcKenzie Jul 23 '14 at 09:39

1 Answers1

1

The way your copy constructor is written, when you copy an object, both objects will be pointing to the same address and both will try to delete it.

Khouri Giordano
  • 1,426
  • 15
  • 17