Class implementation clarification

[hornet07]

I'm somewhat confused about this code I have to work on so I'll just ask for someone to tell me what I did wrong so far. The class declaration is from the course I'm reading, so it stays the same. The implementation is the one I have to write.

// FloatArray.h

#ifndef FLOAT_ARRAY_H
#define FLOAT_ARRAY_H

class FloatArray
{
public:
	// Create a float array with zero elements
	FloatArray();

	// Create a float array with 'size' elements
	FloatArray(int size);

	// Create a FloatArray from another FloatArray 
	// -- be sure to prevent memory leaks!
	FloatArray(const FloatArray& rhs);

	// Free dynamic memory
	~FloatArray();

	// Define how a FloatArray shall be assigned to another FloatArray --
	// be sure to prevent memory leaks!
	FloatArray& operator=(const FloatArray& rhs);

	// Resize the FloatArray to a new size
	void resize(int newSize);

	// Return the number of elements of the array
	int size();

	// Overload bracket operator so client can index
	// into FloatArray objects and access the elements
	float& operator[](int i);

private:
	float* mData; // Pointer to array of floats (dynamic memory)
	int mSize     // The number of elements in the array.
};

#endif // FLOAT_ARRAY_H




// FloatArray.cpp

#include "FloatArray.h"
#include <iostream>
using namespace std;

FloatArray::FloatArray()
{
	mSize = 0;
	mData = 0;
}

FloatArray::FloatArray(int size)
{
	mSize = size;
	mData = new float[mSize];
	mData = 0;
}

FloatArray::FloatArray(const FloatArray& rhs)
{
	float* newArray = new float[rhs.mSize];

	if (rhs.mSize >= mSize)
	{
		for (int i = 0; i < mSize; ++i)
			newArray[i] = rhs.mData[i];
	}
	else
	{
		for (int i = 0; i < rhs.mSize; ++i)
			newArray[i] = rhs.mData[i];
	}
	
	delete[] mData;
	mData = newArray;
}

FloatArray::~FloatArray()
{
	delete[] mData;
	mData = 0;
}

I'm not sure if I'm supposed to create dynamically allocated arrays and then assign them to the mData pointer, or if the mData itself should be used as an array.

[Jaybob66]

Line 60,61 wont be helping, the new'd data is immediately orphaned by line 61 leaving a FloatArray with no data and a memory leak.

At line 69 you dont need to worry about the existing array size as it will be deleted.

eg;

float* newArray = new float[rhs.mSize];
for (int i = 0; i < rhs.mSize; ++i)
   newArray[i] = rhs.mData[i];
delete[] mData;
mData = newArray;

And your dtor should really check if mData is 0 before deleting it, as it possible to create a FloatArray with no data.
eg;

{
   FloatArray fa;
}

[hornet07]

Ok, so these are the modifications as you said.

FloatArray::FloatArray(const FloatArray& rhs)
{
	float* newArray = new float[rhs.mSize];

	for (int i = 0; i < rhs.mSize; ++i)
		newArray[i] = rhs.mData[i];

	delete[] mData;
	mData = newArray;
}

FloatArray::~FloatArray()
{
	if (mData != 0)
	{
		delete[] mData;
		mData = 0;
	}
}

However, I don't entirely understand the problem you mentioned at the previous lines 60, 61. How about this:

FloatArray::FloatArray(int size)
{
	mSize = size;
        float* array = new float[size];
	mData = array;
}

[MikeyBoy]

What's the purpose of the local variable array, when you're immediately going to store its value in mData?

[hornet07]

That's what's bugging me. I don't know if I should use mData as an array or make a new one...

[MikeyBoy]

Are there any reasons why it needs to be dynamically allocated? What is the lifetime of the array? Does it need to be resized?

You said that the class declaration is fixed by the course - does that include the declaration of mData?

	float* mData; // Pointer to array of floats (dynamic memory) 

[hornet07]

Yes, everything in FloatArray.h is as it should be. I need to work on the cpp to accomodate the commented "exercises".

[MikeyBoy]

Well then, you've answered your own question. It should be clear from that line whether or not you need to dynamically allocate an array.

[hornet07]

Well, I managed to write the implementation and there aren't any errors. Unfortunately it doesn't print out what it should. Here's the full code:

// FloatArray.h

#ifndef FLOAT_ARRAY_H
#define FLOAT_ARRAY_H

class FloatArray
{
public:
	// Create a float array with zero elements
	FloatArray();

	// Create a float array with 'size' elements
	FloatArray(int size);

	// Create a FloatArray from another FloatArray 
	// -- be sure to prevent memory leaks!
	FloatArray(const FloatArray& rhs);

	// Free dynamic memory
	~FloatArray();

	// Define how a FloatArray shall be assigned to another FloatArray --
	// be sure to prevent memory leaks!
	FloatArray& operator=(const FloatArray& rhs);

	// Resize the FloatArray to a new size
	void resize(int newSize);

	// Return the number of elements of the array
	int size();

	// Overload bracket operator so client can index
	// into FloatArray objects and access the elements
	float& operator[](int i);

private:
	float* mData; // Pointer to array of floats (dynamic memory)
	int mSize;    // The number of elements in the array.
};

#endif // FLOAT_ARRAY_H



// FloatArray.cpp

#include "FloatArray.h"
#include <iostream>
using namespace std;


FloatArray::FloatArray()
{
	mSize = 0;
	mData = 0;
}

FloatArray::FloatArray(int size)
{
	mSize = size;
	mData = 0;
	mData = new float[mSize];

	for (int i = 0; i < mSize; ++i)
		mData[i] = 0;
}

FloatArray::FloatArray(const FloatArray& rhs)
{
	/*delete[] mData; 
	mSize = rhs.mSize;
	mData = new float[mSize];

	for (int i = 0; i < mSize; ++i)
		mData[i] = rhs.mData[i];*/
	*this = rhs;
}

FloatArray::~FloatArray()
{
	if (mData != 0)
	{
		delete[] mData;
		mData = 0;
	}
}

FloatArray& FloatArray::operator=(const FloatArray& rhs)
{
	mSize = rhs.mSize;
	delete[] mData;

	mData = new float[mSize];
	for (int i = 0; i < mSize; ++i)
		mData[i] = rhs.mData[i];

	return *this;
}

int FloatArray::size()
{
	return mSize;
}

void FloatArray::resize(int newSize)
{
	float* newArray = new float[newSize];

	if (newSize >= mSize)
	{
		for (int i = 0; i < mSize; ++i)
			newArray[i] = mData[i];
	}
	else
	{
		for (int i = 0; i < newSize; ++i)
			newArray[i] = mData[i];
	}

	delete[] mData;

	mData = new float[newSize];
	
	for (int i = 0; i < newSize; ++i)
		mData[i] = newArray[i];

	delete[] newArray;
	newArray = 0;
}

float& FloatArray::operator[](int i)
{
	return mData[i];
}




// FloatArrayDriver.cpp

#include "FloatArray.h"
#include <iostream>
using namespace std;

void PrintFloatArray(FloatArray& fa)
{
	cout << "{";
	for (int i = 0; i < fa.size(); ++i)
		cout << fa[i] << " ";
	cout << "}" << endl << endl;
}


int main()
{
	FloatArray A;

	A.resize(4);
	A[0] = 1.0f;
	A[1] = 2.0f;
	A[2] = 3.0f;
	A[3] = 4.0f;

	cout << "Printing A: ";
	PrintFloatArray(A);

	FloatArray B(A);

	cout << "Printing B: ";
	PrintFloatArray(B);

	FloatArray C = B = A;

	cout << "Printing C: ";
	PrintFloatArray(C);

	A = A = A = A;

	cout << "Printing A: ";
	PrintFloatArray(A);



	cin.ignore();
	cin.get();
}

It SHOULD print

Printing A: { 1 2 3 4 }
Printing B: { 1 2 3 4 }
Printing C: { 1 2 3 4 }
Printing A: { 1 2 3 4 }

But mine prints:

Printing A: {}
Printing B: {}
Printing C: {}
Printing A: {}

Can someone spot my mistake? NOTE: FloatArray.h and FloatArrayDriver.cpp were provided by the course so they must stay the same. The mistakes are obviously in FloatArray.cpp, which I wrote.

[Jaybob66]

resize() needs to set the new mSize. Its still zero from initialisation.

[hornet07]

Thanks for the spot Jaybob. After I changed the mSize to newSize, it looked like this.

Printing A: {1 2 3 4 }
Printing B: {1 2 3 4 }
Printing C: {1 2 3 4 }
Printing A: {-4.31602e+008 -4.31602e+008 -4.31602e+008 -4.31602e+008 }

Fortunately, I checked for self-assignment in the overloaded assignment operator and it worked. Like this, in case someone actually uses the same course:

if (this == &rhs)
		return *this;

Thanks everyone for the help.