C++ converting to std::vector

[sturk]

Hi, I need help in converting my arrays to std::vector in the class polygon as I am facing a memory leak issue due to using the new operators and all that. I tried many times implementing the std::vector, but all I am receiving is just compilation error. I've also used ~Polygon() { delete [] mpt; }, but there is deallocation error. Note: Not allowed to change main(). It's my first time doing std::vector, is anyone able to help? Thank you.

Number of allocations : 4
Number of deallocations : 3

#include <iostream>
#include <cstdlib>
#include <ctime>
#include <cmath>
//#include <vector>

class Object
{
private:
	float d;

public:
	Object(float n) : d(n){}
	Object(){}
 
 	struct PointType
	{
		float x2;
		float y2;
		PointType( float x1,  float y1) :x2(x1),y2(y1){}
		PointType(){}
		float x()
		{
			return x2;
		}
		float y()
		{
			return y2;
		}
		PointType center()
		{
		return *this;
		}
 
	};
	struct VectorType
	{
		float tx;
		float ty;
		VectorType(float tx1,  float ty1) :tx(tx1),ty(ty1){}
		VectorType(){}
	};

	virtual ~Object(){}
};
class Point :public Object 
{
 
private:
    PointType mpoint;
	
public:

    Point(const PointType& pt, float& y1) :Object(y1), mpoint(pt) {}
	Point(const PointType& pt):mpoint(pt){}
	Point(){}
	
  	 Point center() const
	{
	     return *this;
	}
	float x()
	{
		return mpoint.x2;

	}
	float y()
	{
		return mpoint.y2;
	}
};
class Polygon : public Point //specific class with memory leak
{
private:
	// std::vector<Point> pts; 
	Object::PointType *mpt;
	int msize;
	
public:
	
	PointType& operator[](int index)  const
	{	
		return mpt[index];
		
	}
	 
	Polygon(const PointType* points, int npoints):mpt{new Object::PointType[npoints]},msize(npoints)
	{
		for(int i = 0; i < msize; ++i)
		{
			mpt[i]  = points[i];
		}
 	}
	
	Polygon(PointType* points, int npoints, float depth)
	:  Point(*points,depth),mpt{new Object::PointType[npoints]},msize(npoints){
	
		for(int i = 0; i < msize; ++i)
		{	 
		 mpt[i]  = points[i];
 		}
	}
	
	Polygon center() 
	{
	     return *this; 
	}
	
	float x()
	{  	float avg=0;
		float sum=0;
		for(int i = 0; i < msize; i++)
		{
			sum += mpt[i].x2;
			avg = sum / msize;
		}
		return avg;
	}
	
	float y()
	{
		float avg1=0;
		float sum1=0;
		for(int i = 0; i < msize; i++)
		{
			sum1 += mpt[i].y2;
			avg1 = sum1 / msize;
		}
		return avg1;
 	}
	
 	int size() const
	{
		return msize;
	}
 
};
const float EPSILON = 1e-5f;

bool is_near(float x, float y)
{
	return std::abs(x - y) < EPSILON;
}

float frand()
{
	return 10.0f * float(rand()) / float(RAND_MAX);
}

int main()
{
	srand(unsigned(time(0)));
	int count = 0;
	int max_count = 0;

	int max_index = 3 + rand() % 8;
	float* xs = new float[max_index];
	float* ys = new float[max_index];
	float depth = frand();
	float x = 0;
	float y = 0;
	Polygon::PointType* vertices = new Polygon::PointType[max_index];
	for (int index = 0; index < max_index; ++index)
	{
		xs[index] = frand();
		ys[index] = frand();
		vertices[index] = Polygon::PointType(xs[index], ys[index]);
		x += xs[index];
		y += ys[index];
  	}
	x /= static_cast<float>(max_index);
	y /= static_cast<float>(max_index);
	Polygon polygon(vertices, max_index, depth);
	delete[] vertices;
	if (polygon.size() == max_index)
	{
		++count;
	}
	else
	{
		std::cout << "  - Polygon::size test failed" << std::endl;
	}
	++max_count;

	int index = 0;
	while ((index < max_index) &&
		is_near(polygon[index].x(), xs[index]) &&
		is_near(polygon[index].y(), ys[index]))
	{
		++index;
	}
	if (index == max_index)
	{
		++count;
	}
	else
	{
		std::cout << "  - Polygon::operator[] test failed" << std::endl;
	}
	++max_count;

	if (is_near(polygon.center().x(), x) &&
		is_near(polygon.center().y(), y))
	{
		++count;
	}
	
	else
	{
		std::cout << "  - Polygon::center test failed" << std::endl;
	}
	++max_count;
 	delete[] xs;
	delete[] ys;

	int result = static_cast<int>(
		100.0f * static_cast<float>(count) / static_cast<float>(max_count) + 0.5f
	);
	std::cout << "Tests passed: " << result << "%" << std::endl;
	return result;
}

[keskiverto]

For starters:

std::vector<float> xs( max_index );
std::vector<float> ys( max_index );
float depth = frand();
float x = 0;
float y = 0;
std::vector<Polygon::PointType> vertices( max_index );
for (int index = 0; index < max_index; ++index)
{
  xs[index] = frand();
  ys[index] = frand();
  vertices[index] = Polygon::PointType( xs[index], ys[index] );
  x += xs[index];
  y += ys[index];
}

delete[] xs;
delete[] ys;

and

class Polygon : public Point //specific class with memory leak
{
private:
  std::vector<Object::PointType> mpt;
  Object::PointType *mpt;
  int msize;
public:
  Polygon( const std::vector<Object::PointType> & points )
  : mpt( points )
  {
  }

and so forth

[sturk]

Hi keskiverto, sorry i'm not allowed to change the main() forgot to mention.

[zapshe]

If you can't change int main(), how will you convert the arrays into vectors? keskiverto showed you the right path.

If you CAN'T change int main(), then you'll keep your arrays instead of vectors. In this case, best to solve the memory leak issue.

[Niccolo]

I've only skimmed the code, so I may easily have missed something, but....

It appears to me that if you can't change main, you can only modify the polygon class to use a std::vector for internal storage. Since that copies the points it is possible (at least, for your question, that makes it possible because the code doesn't expect Polygon's to take possession of the array passed to the constructor).

@keskiverto's latter code block hints at the change. Since you can't change main(), you must leave the Polygon constructor as it was, but use the new mpt (a std::vector<Object::PointType> ) to receive a copy of that array. After that, you're probably done.

Another observation about Polygon - as it is now, where you use a raw array of PointTypes, there's no Polygon destructor (that I saw when skimming) that would ever delete that array, so there's a memory leak. Moving to a std::vector will eliminate that bug - which I assume SHOULD be part of the "lesson" here.

This is nuts, really, and makes we worry a bit about educational methods. Perhaps they're working on the theory that showing how things should NOT be done will teach through mistakes (like this design) which are fixed by better design. I could only wrap my head around that if the central point is made clear through lecture and process. I mean, my years as a C programmer (before C++ existed) was certainly an education.

[sturk]

Hi Niccolo, thanks for that invaluable advice! How should I go about implementing what you said about "use the new mpt (a std::vector<Object::PointType> ) to receive a copy of that array.?"

[Niccolo]

For Polygon, here's a suggested partial listing:

class Polygon : public Point //specific class with memory leak
{
private:
	std::vector<Object::PointType> pts; 
	//Object::PointType *mpt;
	int msize;
	
public:
	
	PointType& operator[](int index)  const
	{	
        //be careful here, if index is out of range this crashes, index must be < pts.size()
        //since you return a reference, you have to deal with what you return as an error
        //condition - maybe for school you can ignore this, but if you do so on the assumption
        //that only Polygon functions call for this, make it private or skip this function 
        //and just use mpt[index] in those functions

		return mpt[index];
		
	}
	 
	Polygon(const PointType* points, int npoints, float depth )
	{
         // most of us would consider for efficiency
         pts.reserve( npoints );

		for(int i = 0; i < npoints; ++i)
		{
			mpt.push_back( points[i] );
		}
 	}

......the rest of the class....

};

Notes:

- Read comments in operator[]

- The loop in the constructor uses npoints for the limit ( i < npoints )

- Look up std::vector.reserve

- Note the use of std::vector.push_back instead of []

[keskiverto]

Read: http://www.cplusplus.com/reference/vector/vector/vector/

Now you should know that vector has multiple constructors.

One of them is the copy constructor. I did use it in:

Polygon( const std::vector<Object::PointType> & points )
: mpt( points ) // mpt is copy-constructed from points
{
}

Lets write an another Polygon's constructor:

Polygon( const Object::PointType* points, size_t npoints )
: mpt( points, points+npoints )
{
  // do we have to do something here?
}

Can you tell, based on the linked description of vector, which constructor is used and what does it do?

Lets start a different version:

Polygon( const Object::PointType* points, size_t npoints )
{ // we have now default-constructed mpt
  if ( points && 0<npoints ) {
    mpt.reserve( npoints ); // mpt remains empty
  
    // copy here elements from points into mpt
    // use the emplace_back() member of vector
  }
}