[RESOLVED] What's wrong with the assignment operator?

[dan56965]

Hi. Can someone please point out what's wrong with my "=" operator for this class? As it stands, it doesn't seem to do anything.

Any help would be greatly appreciated!

#include <iostream>
#include <string>
#include <fstream>
using namespace std;
//############################################################################################################################################################
//############################################################################################################################################################
//############################################################################################################################################################
class float_seq
{
public:
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
	float_seq(int n=100)																		//CONSTRUCTOR. Accepts an integer as the size of the float_seq array.
	{																							//
		if (n>1)																				//
		{																						//	
			number_of_elements = n;																//
			data = new float[number_of_elements];												//
		}																						//
		else																					//
		{																						//
			cout << "Error: '" << n << "' is not a valid array size" << endl;					//
		}																						//
	}
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
	float_seq(const float_seq &a):number_of_elements (a.number_of_elements)						//COPY CONSTRUCTOR.
	{																							//
		data = new float[number_of_elements];													//
		for(int i=0;i<number_of_elements;i++) data[i] = a.data[i];								//
	}											
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
	~float_seq()																				//DESTRUCTOR
	{																							//
		delete[] data;																			//
	}
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
	float_seq &operator=(const float_seq &a)													//'=' OPERATOR.
	{																							//
		if (this == &a) return (*this);															//
																								//
		if(number_of_elements!=a.number_of_elements)											//
		{																						//
			number_of_elements=a.number_of_elements;											//
			delete[] data;																		//
			data = new float[number_of_elements];												//
		}																						//
	}
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
	float float_seq::read(int i) const															// READ FUNCTION FOR FLOAT_SEQ. "const" makes function read-only.
	{																							//
		float temp=-1;																			//
		if ((i<=(number_of_elements-1)) && (i>=0))												// ...checks that 'i' is a valid array size
		{																						//
			cout << "Read function called:\tArray[" << i << "] = " << data[i] << endl;			// ...prints out array value
			temp = data[i];																		//
		}																						//
																								//
		else																					// ...else if "i" is OOB, user is told
		{																						//
			cerr << "Error: '" << i << "' is not a valid array reference" << endl;				//
		}																						//
																								//
		return temp;																			// ...returns value, or -1 if input was invalid.
	}	
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
	float float_seq::write(int i, float input)													// WRITE FUNCTION FOR FLOAT_SEQ.
	{																							//
		float temp=-1;																			//
		if ((i<=(number_of_elements-1)) && (i>=0))												//
		{																						//
			data[i] = input;																	//
			cout << "Write function called:\tArray[" << i << "] = " << data[i] << endl;			//
			temp = 0;																			//
		}																						//
																								//
		else																					//
		{																						//
			cerr << "Error: '" << i << "' is not a valid array reference" << endl;				//	
		}																						//
																								//
		return temp;																			// 
	}	
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
	friend istream &operator>>(istream&,float_seq&);											// STREAM OPERATORS
	friend ostream &operator<<(ostream&,float_seq&);											// friend declarations for
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
private:																						// PRIVATE OBJECTS																					//
	int number_of_elements;																		// number of elements in float_seq
	float *data;																				// pointer to first element in float_seq
};

ostream &operator<<(ostream &out, float_seq &c)
{
	out<<"\nArray elements are:\n";

	for (int i=0;i<c.number_of_elements;i++)
	{
		out << "data[" << i << "]\t=\t" << c.data[i] << endl;
	}

	out << "\n\nSize is:\t" << c.number_of_elements << "\n\n";
	return (out);
}
//############################################################################################################################################################
//############################################################################################################################################################
//############################################################################################################################################################
int main(void)
{
	float_seq V(10);							//creates 'V'
	V.zero();


	V.float_seq::write(3,(float) 3.1415);		// sets array[3] in 'V' to '3.1415'
	

	cout<<V;

	float_seq W(V);								// creates new copy of 'V', called 'W'
	

	W.float_seq::write(3, (float) 1.602);		// sets array[3] in 'W' to 1.602
	cout<<W;

	W=V;										// assigns 'V' to 'W'
	cout<<W;

	cin.get();

}

[Skizmo]

The '=' operator doesn't return something. It also deletes the internal data but doesn't copy the data from the other object.

[dan56965]

Thanks for the reply!
I think I've got it working properly now, but if you could glance over it and check I'd be grateful.

	float_seq &operator=(const float_seq &a)					//'=' OPERATOR.
	{										//
		if (this == &a) return (*this);						// if 'b'=='a', return pointer to 'b'.
											//
		else									// else if b!=a,
		{									//
			number_of_elements=a.number_of_elements;			// ...change number_of_elements to reflect source array
			delete[] data;							// ...and free space used for 'b'
											//
			data = new float[number_of_elements];				// ...create new array of right size
			for(int i=0;i<number_of_elements;i++) data[i] = a.data[i];	// ...copy data across
			return (*this);							// ...return pointer to newly allocated array
		}									//
	}

Thanks again.

[Paul McKenzie]

Thanks for the reply!
I think I've got it working properly now, but if you could glance over it and check I'd be grateful.

The code is not exception safe. What happens if new[] threw an exception (out of memory)? You've gone ahead and deleted the data, making the object useless.

For this type of code, you should always allocate first, then delete second. Since allocation can fail, you have to make sure you don't destroy your data just in case the failure occurs.

float_seq &operator=(const float_seq &a)
{
    if (this == &a) 
         return (*this);	
    else
    {
        float *tempData = new float[number_of_elements];
        number_of_elements=a.number_of_elements;
        delete [] data;
        for(int i=0;i<number_of_elements;i++) 
            tempData[i] = a.data[i];
       data = tempData;
       return (*this);
    }
}

Note that the call to new[] is done first and saved to a temporary. If new[] were to fail, then the original data was never deleted. Then the data[] is deleted, since at this point, it is safe that no C++ exception can be thrown.

Then at the end, I assign the data by assigning the tempData pointer.

Regards,

Paul McKenzie

[dan56965]

I like it!! ... That makes a lot of sense.
Paul, Skizmo - Thank you both for your comments!