Returning an object with a dynamic pointer

If I have an class with a dynamically allocated array inside of it, how can I return an object from an overloaded addition operator?

The class is basically:

Code:

class foo
{
   foo& operator + (foo & myFoo);
   int *myints;
};

Assume that all myints pointers are dynamically allocated to being 10 integers, which have values...

Then in the addition I tried

Code:

foo& foo::operator+ (foo & myFoo)
{
   foo mytemp; //assume mytemp's myints also is correctly allocated as per above...
   for (int a=0; a < 10; a++)
      { *(temp.myints + a) = *(myFoo.myints + a) + *(this->myints + a);
      }
   return temp;
}

When I try code like this (sorry I don't have the exact code-- I'm on a my laptop at the moment...) I get a warning that I'm returning a reference to a local variable. Is there any way to fix this??

thanks!

hmm... but you are returning by value, not by reference.

By the way, you should consider using a std::vector<int> instead of an int*. You could consider implementing operator+= as a member function, then implement binary operator+ as a non-member function using operator+=.

laserlight,

I would love to use a vector, but this little snippet is part of a course I'm taking (universities love to reinvent the wheel for assignments

What I can't seem to figure out is this type of code works fine if the class elements are already defined (for instance, if I was using an array, instead of a pointer), but blows up when I try to do it this way :P

Originally Posted by maxsthekat

What I can't seem to figure out is this type of code works fine if the class elements are already defined (for instance, if I was using an array, instead of a pointer), but blows up when I try to do it this way :P

Post the smallest and simplest program that demonstrates the problem.

EDIT:
Oh, I see that you have edited your original post. In that case, the answer is simple: change it back It makes sense to have operator+ return by value. Additionally, if you really do want operator+ to be a member, then it should be a const member function. You should also provide it with a const reference parameter since it will not modify what is passed to it.

I thought I did that with the snippet, although the snippet will not compile by itself... Here is the full:

PtrandRef.h

Code:

#include <iostream>


using namespace std;

class PtrandRef{
private:
	int maxSize;
	int length;
	int *elementArray;

public:
	PtrandRef();                                  //Default Constructor
	PtrandRef(int size = 10);                     //Constructor with parameter
	PtrandRef(const PtrandRef& obj);              //Copy Constructor

	~PtrandRef();                                
	//Destructor
    //Delete all the elements from the array
	//Postcondition: Memory allocated for the array is released


	void print() const;                
	//Function to output array
	//Precondition:The array must exist
	//Postcondition: The elements of the array are printed on the standard output device

    const PtrandRef& operator=(const PtrandRef&);
	//Function to overload the assignment operator

    const PtrandRef& operator+        
        	(const PtrandRef&);
   //overload the operator +
   //Precondition: The array must exist and the length is 10
   //Postcondition: The sum of elements in the array is computed

    void removeAt(int location);
	//Function to delete value from the array
	//Precondition: The array must exist and the length is 10
	//Postcondition: If found, element is deleted from the array and the length is decremented by one;otherwise, an appropriate message is printed

    void changeAt(int location, int value);
	//Function to change an element in the array at a certain location to another given value
	//Precondition: The array must exist and the length is 10
	//Postcondition: the element of the array in a certain location is changed to another given value, and the new array is displayed
	//if the position is out of range, print error message, and the elements in the array are unchanged

    void insertAt(int location, int value);
	//Function to insert newItem in the array
	//Precondtion: The array must exist and must be within the length of 10
	//Postcondition: new value is inserted in the array and the length is increamented by one.

    int minimumAt();
	//Function to compute the minimum value of the array
	//Precondition: The array must exist and the length is 10
	//Postcondition: the minimum value is returned

    void sort();
	//Function to sort the values in the array in an increasing order
	//Precondition: The array must exist and the length is 10
	//Postcondition: Values are sorted in an increasing order in the array and the array length is unchanged.

};

PtrandRef.cpp

Code:

#include <iostream>
#include <iomanip>
#include <cstring>
#include <cassert>
#include "PtrandRef.h"

using namespace std;


	PtrandRef::PtrandRef()
	{

	}

	PtrandRef::PtrandRef(int size)
	{
            length = 0;
            if(size <= 0)
            {
            	cout << "Array size must be positive." << endl;
            	cout << "Creating an array of size 10." << endl;
            	maxSize = 10;
            }
            else
            	maxSize = size;

            elementArray = new int[maxSize];
	}

	PtrandRef::PtrandRef(const PtrandRef& obj)
	{
		maxSize = obj.maxSize;
		length = obj.length;
		elementArray = new int[maxSize];

		for(int i = 0; i < maxSize; i++)
			*(elementArray + i) = *(obj.elementArray + i);

	}

	PtrandRef::~PtrandRef()
	{
		//To be implemented
		delete [] elementArray;
	}



	void PtrandRef::print() const
	{
             if(length == 0)
            	 cout << "Array is empty." << endl;
             else
             {
            	 for(int i = 0; i < length; i++)
            		 cout << *(elementArray + i) << " ";
            	 cout << endl;
             }
	}

    const PtrandRef& PtrandRef::operator=
    	(const PtrandRef& otherArray)
    {
    	if(this != &otherArray)            //avoid self-assignment
    	{
    		if(elementArray != NULL)
    		{
    			delete[] elementArray;
    			elementArray = NULL;
    			maxSize = 0;
    			length = 0;
    		}
    		maxSize = otherArray.maxSize;
    		length = otherArray.length;

    		if(maxSize != 0)
    		{
    			elementArray = new int[maxSize];

    			for(int i = 0; i <length; i++)
    				*(elementArray + i) = *(otherArray.elementArray + i);
    		}

    		else
    			elementArray = NULL;
    	}
    	return *this;

    }

    const PtrandRef& PtrandRef::operator+
        	(const PtrandRef& inPtr)
        	{
                   PtrandRef  temp(10);
		   temp = inPtr;
		   //int temp = 0;
				if (length != 10)
				{
					   cout << "The array length is not 10" << endl;
				}
				else 
				{
				   
				   	
				   for (int a = 0; a < length; a++)
					   {
						   *(temp.elementArray + a) = *(elementArray + a) + *(inPtr.elementArray + a);
					//sum += *(elementArray + a) + *(inPtr.elementArray + a);
					   }
				}
				
				return temp;
				//return sum;
        	}

   

    void PtrandRef::insertAt(int location, int value)
    {
    	if(location < 0 || location >= maxSize)
    	        	  cerr << "The location of the item to be inserted is "
    	        	       << "out of range." << endl;
    	          else
    	        	  if(length >= maxSize)      //array is full
    	        		  cout << "Cannot insert in a full list. " << endl;
    	        	  else
    	        	  {
    	        		  for(int i = length; i > location; i--)
    	        			  *(elementArray + i) = *(elementArray + i - 1);

    	        		  *(elementArray + location) = value;

    	        		  length ++;
    	        	  }
    }

    void PtrandRef::removeAt(int location)
    {
         if (length != 10)
		 {
			 cerr << "The array length is not 10" << endl;
		 }
		 else 
		 {

			 for (int i = location; i < maxSize - 1; i++)
			 {
				 *(elementArray + location) = *(elementArray + location + 1);
			 }
			 length--;
		 }


    }

    void PtrandRef::changeAt(int location, int value)
    {
          if(location < 0 || location >= length)
        	  cerr << "The location of the item to be replaced is "
        	       << "out of range." << endl;
          else
        	  *(elementArray + location) = value;
    }

    int PtrandRef::minimumAt()
    {
        int min = 0;	//temp variable for holding minimum   
		if (length != 10)
		{
		   cerr << "Array length is not 10. Cannot compute minimumAt" << endl;
		}
		else
		{
			min = *elementArray; //set to the first element
			for (int i = 0; i < length; i++)
			{
				if (*(elementArray + i) < min)
					min = *(elementArray + i);
			}
			return min;
		}

    }

    void PtrandRef::sort()                         //extra credits
    {
           if (length != 10)
		   {
			   cerr << "Sort: Array length is not 10. Cannot compute." << endl;
		   }
		   else
		   {
				int temp;	
			   for (int i=0; i < length; i++)
				{
					for (int j= i+1; j < length; j++)
					{
						if (*(elementArray + i) > *(elementArray + j))
						{
							temp = *(elementArray + j);
							*(elementArray + j) = *(elementArray + i);
							*(elementArray + i) = temp;
						}
					}
			   }
		   }
    }

Driver.cpp

Code:

#include <iostream>
#include "PtrandRef.h"

int main()
{
	PtrandRef array(10);
    PtrandRef array1(10);        
	PtrandRef array2(10);

	int counter;
	int number;

	int value;
	int location;

	cout << "Enter 10 integers: ";

	for(counter = 0; counter < 10; counter++)
	{
		cin >> number;
		array.insertAt(counter, number);
	}

	cout << endl;

	cout << "The array you entered is: ";
	array.print();
	cout << endl;

        array1 = array;
        cout <<"After assigning array to array1, array1 is:"<<endl;
        array1.print();


	cout << "Enter the value to be changed to: ";
	cin >> value;
	cout << endl;
	cout << "Enter the position to be changed: ";
	cin >> location;
	array.changeAt(location, value);
	cout << "After change the value at location "<< location <<" to value"<< value <<", the array is: "<<endl;
	array.print();
	cout << endl;

	cout << "Array + Array1 = ";
	(array + array1).print();



	cout << "Minimum of array = " << array.minimumAt() << endl;

	cout << "Sorted array: ";
	array.sort();
	array.print();

	cout << "Enter position to remove value: ";
	cin >> value;
	array.removeAt(value);
	array.print();

}

Don't worry about the commented out lines in the operator+... It's just there in case they wanted us to do something else and I have to switch the code over....

Originally Posted by laserlight

hmm... but you are returning by value, not by reference.

He's returning a reference to a local variable. That's bad...

Your operator function should probably look more like:

Code:

foo& foo::operator+ (foo & myFoo)
{
   for (int a=0; a < 10; a++)
      { *(this->myints + a) = *(myFoo.myints + a) + *(this->myints + a);
      }
   return *this;
}

Ah, refer to my edit of my previous post. It still applies.

Originally Posted by maxsthekat

I thought I did that with the snippet, although the snippet will not compile by itself... Here is the full:

Note "smallest and simplest". That said, this gives more opportunity for improvement

• Your header file should have inclusion guards.
• Do not place using directives (e.g., using namespace std) or using declarations (e.g., using std::cout) at namespace scope, especially global scope, in header files. The inclusion of the header can lead to it being propagated across to source files where it is not correct.
• Your default constructor probably should do something.
• Instead of such a complicated copy assignment operator implementation, consider providing a member swap function, then implement copy assignment using the copy constructor, member swap and destructor.
• Instead of *(elementArray + i), make use of syntactic sugar: elementArray[i].
• If you have been taught them or are otherwise allowed to use them, consider throwing exceptions where appropriate instead of just printing an error message in the function itself.

EDIT:

Originally Posted by bithub

He's returning a reference to a local variable. That's bad...

Obviously. Notice the edit.

Originally Posted by bithub

Your operator function should probably look more like:

No, it should not. That is more of the semantics of operator+= than of operator+.

bithub,

Thanks for the reply!

Here's what I ran into when I did that: the first object (this) is now altered by the operation (which makes it a +=, instead of a + operator) say you have code like:

Code:

foo foo1; // say myints is 1, 2, 3, 4, 5, 6, 7, 8, 9, 10
foo foo2;// say myints is 3, 4, 5, 6, 7, 8, 9, 10, 11, 12

foo1 + foo2; //stores 4, 6, 8, 10, 12, 14, 16, 18, 20, 22 in foo1's myints

EDIT:

Laserlight, ah, I see what you're saying now Sorry about the long posting. The bulk of this code isn't mine-- we're just filling in the functions, but thanks for the advice! I'll be sure to do that in my code

We're not allowed to use elementArray[i]... Believe me, I'd love to do that instead!

Can you elaborate a bit more on what you mean about the copy constructor?

Thanks!

Originally Posted by maxsthekat

Here's what I ran into when I did that: the first object (this) is now altered by the operation (which makes it a +=, instead of a + operator) say you have code like:

Yes, it is good that you notice the semantics. I repeat: change it to return by value.

Originally Posted by laserlight

No, it should not. That is more of the semantics of operator+= than of operator+.

Yep, my mistake. Wasn't paying close enough attention.

Originally Posted by maxsthekat

We're not allowed to use elementArray[i]... Believe me, I'd love to do that instead!

Fair enough, misguided instructors are terribly irritating

Originally Posted by maxsthekat

Can you elaborate a bit more on what you mean about the copy constructor?

Yes. Read GotW #59: Exception-Safe Class Design, Part 1: Copy Assignment.

Eureaka!

Thanks for the help, guys! I read the link, then wrote a copy constructor (then noticed the program already had one-- still, was good practice...) and did as you said for returning by value. It works perfectly. Thanks for your help and your patience with my thick skull