CODEWITHSUNDEEP
c++
user3458575
user3458575

Reputation: 1

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

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?

Upvotes: 0

Views: 524

Answers (3)

jfly
jfly

Reputation: 7990

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.

Upvotes: 0

Mike Seymour
Mike Seymour

Reputation: 254431

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.

Upvotes: 1

Alan
Alan

Reputation: 3417

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;
}

Upvotes: 0

Related Questions