Returning an object with a pointer member variable in C++

Question

So I have an upDate object that has a pointer member variable that just points to an array that defaults to May 11, 1959:

upDate::upDate()
{   dptr = new int[3];
    dptr[0] = 5;
    dptr[1] = 11;
    dptr[2] = 1959;
}

Now in my main I'm supposed to override the + operator.

upDate D1(5,9,1994);
upDate D2 = D1+1;//supposed to add 1 more day to the upDate.

But when I output D2, it doesn't have me 5/10/1994. It gives me a really large negative number: -572662307/-572662307/-572662307.

Here's my operator+ method:

upDate upDate::operator+(int integer){
    upDate temp;
    int m = dptr[0];
    //cout << m << endl;
    int d = dptr[1];
    int y = dptr[2];
    int julian = convertToJulian(y, m, d);
    julian += integer;
    //cout << julian << endl;
    temp.convertToGregorian(julian);
    //cout << temp.getMonth() << " " << temp.getDay() << " " << temp.getYear() << endl;
    return temp;
}

All the couts in that method were just me testing to see if it's correct, and they are. So, I don't think any of these are incorrect. However, when I return the temp, I think something gets lost along the way of returning the temp as the D2. Maybe it's a pointer issue? I'm not really sure.

Edit: here's my operator=:

upDate upDate::operator=(upDate copy) {
for(int i = 0; i<3; i++)
    copy.dptr[i] = dptr[i];
return copy;
}

and my destructor:

upDate::~upDate()
{
    delete []dptr;
}

I guess I never made a copy constructor...But I'm confused on how to make it?

You have read about [the rule of three](https://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29)? — Some programmer dude, Mar 25 '14 at 08:14
Have you overloaded the operator =? And show your copy constructor. — jfly, Mar 25 '14 at 08:15
Pileborg and jfly's comments are important, don't ignore them. You can be using assignment and copy constructor without realising (e.g. you're returning a copy of your *temp* object). And in your case, where you have an old-style array of int, you could have a few surprises. Possibly your problem is due to the absence of one of these member functions. — DrD, Mar 25 '14 at 08:25
I posted my operator= and destructor. But I don't think I've made my copy constructor. I'm confused on how I would actually make it... — user3458575, Mar 25 '14 at 08:37

score 1 · Answer 1 · edited May 23 '17 at 11:49

Maybe it's a pointer issue?

Probably. In your update, you say "I guess I never made a copy constructor", which means that the class breaks the Rule of Three and has invalid copy semantics. When it's copied (as it is when returning from the function you posted), both copies contain a pointer to the same array; presumably, both have a destructor which deletes the array, leaving the other object's pointer dangling.

I guess I never made a copy constructor...But I'm confused on how to make it?

The best way to make it is not to. Stop using new and manually juggling pointers, and build your class from correctly copyable objects; in this case, replace dptr with an array, or three named variables. (If you needed a dynamically-sized array, which you don't here, then std::vector would be ideal).

I would probably simplify it further by only storing the Julian day, and only converting to Gregorian when necessary for human-readable presentation.

If you really feel the need to manage memory by steam, then you'll need a destructor

~upDate() {delete [] dptr;}

a copy constructor

upDate(upDate const & other) : dptr(new int[3]) {
    std::copy(other.dptr, other.dptr+3, dptr);
}

and a copy-assignment operator. It should modify the object it's called on, not its argument (and certainly not a local copy of its argument, as yours does), and should conventionally return a reference to the assigned object, not a copy of it.

upDate & operator=(upDate const & other) {
    std::copy(other.dptr, other.dptr+3, dptr);
    return *this;
}

In C++11, you might also want to consider move semantics for the sake of efficiency.

upDate(upDate && other) : dptr(other.dptr) {
    other.dptr = nullptr;
}

upDate & operator=(upDate && other) {
    if (this != &other) {   // Careful: self-assignment can be tricky
        dptr = other.dptr;
        other.dptr = nullptr;
    }
}

Disclaimer: this code was written without the help of a compiler and test framework, so might contain mistakes. Don't use it without testing.

tl;dr Memory management is hard. Don't do it unless you really need to. You don't need to here.

Well, part of the assignment was to use a pointer data member that points to an integer array. Before, this used 3 integer data members (month, day, year), but I had to change it. But thanks. Putting in the copy constructor made it work now. — user3458575, Mar 25 '14 at 09:00

score 0 · Answer 2 · answered Mar 25 '14 at 08:42

Your operator= function works backwards.

//This version tries to make a=b work as though it were b=a
upDate upDate::operator=(upDate copy) {
  for(int i = 0; i<3; i++)
    copy.dptr[i] = dptr[i];
  return copy;
}

That is, judging by the examples I have seen the operator= should copy the values of the parameter into the current object, where you have it copying from the current object to the parameter.

//Corrected version (judging by linked article.)
upDate & upDate::operator=(upDate copy) {
  for(int i = 0; i<3; i++)
    dptr[i] = copy.dptr[i];
  return *this;
}

jfly · Answer 3 · 2014-04-04T02:01:05.877

I guess I never made a copy constructor

NO. Note this line upDate D2 = D1+1;, it would call copy constructor instead of operator =, since you don't specify one, it would call the compiler-generated copy constructor which performs a member-wise copy, so the data member D2.dptr = temp.dptr, they share the same address. After temp is destroyed, the D2.dptr points to an invalid address, so it gives you garbage value.

Returning an object with a pointer member variable in C++

3 Answers3