vector::erase with pointer member

Question

I am manipulating vectors of objects defined as follow:

class Hyp{ public: int x; int y; double wFactor; double hFactor; char shapeNum; double* visibleShape; int xmin, xmax, ymin, ymax; Hyp(int xx, int yy, double ww, double hh, char s): x(xx), y(yy), wFactor(ww), hFactor(hh), shapeNum(s) {visibleShape=0;shapeNum=-1;}; //Copy constructor necessary for support of vector::push_back() with visibleShape Hyp(const Hyp &other) { x = other.x; y = other.y; wFactor = other.wFactor; hFactor = other.hFactor; shapeNum = other.shapeNum; xmin = other.xmin; xmax = other.xmax; ymin = other.ymin; ymax = other.ymax; int visShapeSize = (xmax-xmin+1)*(ymax-ymin+1); visibleShape = new double[visShapeSize]; for (int ind=0; ind<visShapeSize; ind++) { visibleShape[ind] = other.visibleShape[ind]; } }; ~Hyp(){delete[] visibleShape;}; };

When I create a Hyp object, allocate/write memory to visibleShape and add the object to a vector with vector::push_back, everything works as expected: the data pointed by visibleShape is copied using the copy-constructor.

But when I use vector::erase to remove a Hyp from the vector, the other elements are moved correctly EXCEPT the pointer members visibleShape that are now pointing to wrong addresses! How to avoid this problem? Am I missing something?

Is there a reason you can't re-define visibleShape as: std::vector<double> visibleShape, and delete your copy ctor completely? — Jerry Coffin
– Jerry Coffin, Commented Apr 20, 2010 at 18:56
You don't seem to initialize xmin, xmax, etc. or allocate visibleShape in the default constructor but you use calculation on xmin, xmax, etc. to determine that you can read from the other object's visibleShape` in the copy constructor. — Lara Bailey
– Lara Bailey, Commented Apr 20, 2010 at 18:59
Also, all your data members are public so you have no control over whether it's valid to delete[] visibleShape in your destructor; it could have been assigned to anything. You should consider using a vector or at least make it private so that you have a chance to enforce your class invariants. — Lara Bailey
– Lara Bailey, Commented Apr 20, 2010 at 19:08
@Jerry Coffin: I'm using visibleShape to store portions of image of size determined by the other members of Hyp. Isn't it more efficient to use a simple pointer instead of a vector? I am new to C++ so I am open to suggestions! — matt
– matt, Commented Apr 20, 2010 at 19:12
@matt: If there's a difference in efficiency it's going to be tiny - the vector is just doing automatically the stuff you otherwise need to do manually... remember you can use the vector constructor that takes a size parameter, or the resize method, if you know the size in advance (or reserve if you want to use push_back). — Mike Dinsdale
– Mike Dinsdale, Commented Apr 20, 2010 at 19:20

Mike Dinsdale · Accepted Answer · 2010-04-20 18:56:34Z

2

I think you're missing an overloaded assignment operator for Hyp.

answered Apr 20, 2010 at 18:56

Mike Dinsdale

1,5319 silver badges8 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Andreas Brinck · Accepted Answer · 2010-04-20 18:58:18Z

1

I think you might be missing the assignment operator = in the Hyp class.

Hyp& operator = (const Hyp& rhs);

answered Apr 20, 2010 at 18:58

Andreas Brinck

52.9k15 gold badges87 silver badges114 bronze badges

1 Comment

arjacsoh Over a year ago

How does the implementation of this function look like? It is obviously necessary to make use of the copy constructor, isn't it?

Collectives™ on Stack Overflow

vector::erase with pointer member

2 Answers 2

Comments

1 Comment

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

1 Comment

Linked

Related