Returning an object that contains a pointer to data

[KLoux]

Hello all!

I have a class like this:

class MyClass
{
public:
    MyClass(int Count)
    {
        Data = new double[Count];
    };
 
    ~MyClass()
    {
        delete [] Data;
    }
 
    // Some methods to set/get the Data, modify the data,
    // a complete set of overloaded operators, etc.
 
    const MyClass operator + (const MyClass &Target) const
    {
        MyClass Temp(Count);
        Temp = *this;
 
        // The addition algorithm is here...
 
        return Temp;
    }
 
private:
    double *Data;
};
 

Of course, something like this

MyClass obj1(4), obj2(4), obj3(4);
obj3 = obj1 + obj2;// Can't do this!
 

will cause a crash because the Temp::Data is freed when operator+ returns and the assignment operator then tries to access Data (yeah, I didn't show it, but it does).

For a short time, I added a boolean class member called DeleteElements, and changed the destructor to this:

    ~MyClass()
    {
        if (DeleteElements)
            delete [] Data;
    }
 

and I change the operators to something like this

    MyClass operator + (MyClass &Target) const
    {
        MyClass Temp(Count);
        Temp = *this;
 
        // The addition algorithm is here...
 
        Temp.DeleteElements = false;
 
        return Temp;
    }
 

and then you need to very carefully manage the DeleteElements flag to avoid memory leaks. Obviously, this is undesirable (but it works). Besides these reasons, I want to avoid using this method because I cannot use the correct versions of the overloaded operator function signatures (with the consts).

Has anyone else run into these problems? Is there an elegant solution? I'd like to avoid completely re-writing the class, but I can if that's the most effective way to handle the problem.

Thanks for the help!

-Kerry

[newb16]

Looks like shared pointer (shared_ptr).

the correct versions of the overloaded operator function signatures (with the consts).

There is mutable keyword and const_cast.

[Banfa]

Because you have not defined a copy constructor or an operator= for your class the compiler creates them for you. However the compiler creates a shallow copy algorithm, this just copies the members of the source object to the destination object and both objects end up pointing to the same memory block.

You need a deep copy, a deep copy rather than copy the members directly is intelligent enough to allocate a new buffer for the destination object and copy the data pointed to rather than the pointer. You have to implement these yourself by implementing a copy constructor and a operator= but you would not need to modify any existing members of the class.


However at least 2 other methods to solve the problem occur to me. Your class basically holds an array of doubles of variable size and allows one array to be added to another. Rather than the method above you use you could

1. Use a template class. The template parameter would be the size of the array. You would not need to new or delete any data so a shallow copy would work
   
   

   template<int SIZE>
   class MyClass
   {
   ...
    
   private: 
       double Data[SIZE];
   }
    

   If you needed to you could add a helper function to add classes with different sized arrays and if you didn't need to the compiler would raise an error if you tried. The size of the array becomes part of the class definition rather than a parameter of the class. This would prevent easy resizing of the array though.
2. The second option is use a vector. If you ever find yourself using an array or using a pointer and new to dynamically create an array then you should be asking yourself "Why am I not using a vector?" If you have no good reason then you should use a vector. If you used a vector in place of your pointer in the class as is it would work because a vector knows how to copy itself and a shallow copy of your class would work.

The main point is you are trying to do a lot of work manually that the STL or the compiler will do for you and that is always a mistake.

[KLoux]

Banfa, thanks for the info.

Just for my own knowledge - what (in your opinion?) would be a "good reason" for not using a std::vector instead of an array?

And you are right about my not having defined a copy constructor, but I did define an assignment operator that performs a deep copy (it's just not shown). I'm not clear on whether or not adding a copy constructor that performs a deep copy would fix my problem... would the copy constructor be called in the following statement?

MyClass obj1, obj2, obj3;
obj1 = obj2 + obj3;
 

This should be the same as (I think)

obj1.operator=(obj2.operator+(obj3));
 

... no copy constructor. And the memory is lost when operator+ returns before operator= is even called, so the fact that operator= does a deep copy doesn't matter, as the leak has already occured. Am I wrong?

I've considered using a template, but I'm not sure that will work for me either. What I'm really doing is writing a simple matrix algebra class (I know it's been done, but I wanted to give it a shot myself). My goal was to allow for arbitrarily sized matricies (which a template would do), but say I had something like this:

Matrix<4, 3> fourByThreeMatrix;
Matrix<3, 5> threeByFiveMatrix;
Matrix<4, 5> fourByFiveMatrix = fourByThreeMatrix * threeByFiveMatrix;
 

Would it work? I'm not sure I could overload operators to handle multiplication by other arbitrarily sized matricies.

It sounds like the std::vector approach is the best way for me to go.

Thanks again for the help!

-Kerry

[Banfa]

@KLoux
Off the top of my head in a purely C++ program I can't think of a standard generic reason, it would have to be extremely situation specific. If you had to interface to some legacy C code there is more likely to be an instance of needing to use a pointer or array (but note that for single dimensioned arrays vectors maintain the same memory map).

@KLoux
Your code is right but there is an omission in your understanding of what this code does.

Your operator+ returns an object (a temporary one that is on the stack). In order to do this the compile constructs an invisible temporary object from the return value which is guaranteed to still exist after the function call has returned (your temp variable is guaranteed not to exist at this point). This temporary object is then used by the calling code to access the return value The copy constructor is called while creating this hidden temporary object implicit in the return from operator+ causing your error.

This is one of the places where C++ is poor in my opinion. It is too easy to make the mistake you have made and not obvious what the cause is (until you know of course).


As to you matrix class you may not be able to write class member functions for things like multiplication but you may be able to write a helper function to do it. A helper function is a normal function made to operate with a class without actually being a part of the class for instance this

matrix<int N, int M> operator*(matrix<int N, int Q>m1, matrix<int Q, int M>m2)
{
    ...
}
 

might be possible, and if it is would enforce the correct input and output parameter types.

Note I say might, I am not actually sure on this point.

Otherwise vectors rather than pointers and dynamic arrays is a much better idea.

[KLoux]

Great, thanks for the info - This is a big help!

-Kerry

[Banfa]

I should have said in my previous post that having a copy constructor and operator= normally goes hand in hand. If you need one then usually you need the other.