Sign in to follow this  
black_darkness

Can you create a Vector of structs in C++? I need help with a unknown error pertaining to this subject.

Recommended Posts

black_darkness    280
I am painting 64 squares onto a screen using Fast Light ToolKit. In order to save time I created a loop to do this automatically. Since I need to dynamically create new objects I decided to push the struct Rectangle into a vector. However when I run the program I get a single error.
[CODE]error C2248: 'Graph_lib::Shape::Shape' : cannot access private member declared in class 'Graph_lib::Shape'[/CODE]


Here is the main loop
[CODE]
int main()
{
Point tl(100,100); //top left corner of window
Simple_window win(tl,400,400,"My Window");
vector<Rectanglee> rects; //Here is where I think the problem lies.

for (int i=0;i<8;i++){
int xa=(i*50)-1;
for(int j=0;j<8;j++){
int ya=(j*50)-1;
Rectanglee rect(Point(i,j),50,50);
rects.push_back(rect);
}
}
for(int i=0;i<64;i++){
win.attach(rects[i]); //attach rect to screen
}
win.wait_for_button(); //wait until user clicks the next button to close window
}
[/CODE]



Here is the rectanglee struct.
[CODE]
struct Rectanglee : Shape {
Rectanglee(Point xy, int ww, int hh) : w(ww), h(hh)
{
add(xy);
if (h<=0 || w<=0) error("Bad rectangle: non-positive side");
}
Rectanglee(Point x, Point y) : w(y.x-x.x), h(y.y-x.y)
{
add(x);
if (h<=0 || w<=0) error("Bad rectangle: non-positive width or height");
}
void draw_lines() const;
int height() const { return h; }
int width() const { return w; }
private:
int h; // height
int w; // width
};
[/CODE]

Share this post


Link to post
Share on other sites
black_darkness    280
[quote name='Hodgman' timestamp='1355282192' post='5009672']
That error says that the [font=courier new,courier,monospace]Shape[/font] constructor is private. Can you post the code for [font=courier new,courier,monospace]Shape[/font]?
[/quote]

Yes.

Here are the headers I am using http://www.stroustrup.com/Programming/Graphics/ . The shape is in the graph.h file I think.


Anyways here is the Shape class.
[CODE]
class Shape { // deals with color and style, and holds sequence of lines
public:
void draw() const; // deal with color and draw lines
virtual void move(int dx, int dy); // move the shape +=dx and +=dy
void set_color(Color col) { lcolor = col; }
Color color() const { return lcolor; }
void set_style(Line_style sty) { ls = sty; }
Line_style style() const { return ls; }
void set_fill_color(Color col) { fcolor = col; }
Color fill_color() const { return fcolor; }
Point point(int i) const { return points[i]; } // read only access to points
int number_of_points() const { return int(points.size()); }
virtual ~Shape() { }
protected:
Shape();
virtual void draw_lines() const; // draw the appropriate lines
void add(Point p); // add p to points
void set_point(int i,Point p); // points[i]=p;
private:
vector<Point> points; // not used by all shapes
Color lcolor; // color for lines and characters
Line_style ls;
Color fcolor; // fill color
Shape(const Shape&); // prevent copying
Shape& operator=(const Shape&);
};
[/CODE] Edited by black_darkness

Share this post


Link to post
Share on other sites
alvaro    21247
It looks like whoever wrote Shape didn't want them to be copied, but you are making a vector of Rentanglees, which implies copying them around, and the copy constructor for Rectanglee tries to invoke the copy constructor for Shape.

EDIT: Ninja'd. Edited by Álvaro

Share this post


Link to post
Share on other sites
uglybdavis    1065
May i suggest (read the comments, i changed them:

[source lang="cpp"]int main() {
Point tl(100,100);
Simple_window win(tl,400,400,"My Window");
vector<Rectanglee*> rects; // Pointer Magic!

for (int i=0;i<8;i++){
int xa=(i*50)-1;
for(int j=0;j<8;j++){
int ya=(j*50)-1;
rects.push_back(new Point(i,j),50,50);
}
}
for(int i=0;i<64;i++){
win.attach(*(rects[i])); //Remember to de-reference
}
win.wait_for_button(); //wait until user clicks the next button to close window

// Clean up at some point!
for (unsigned int i = 0, size = rects.size(); i < size; ++i)
delete rects[i];
}[/source]

Share this post


Link to post
Share on other sites
Cornstalks    7030
[quote name='uglybdavis' timestamp='1355283813' post='5009685']
May i suggest (read the comments, i changed them:
*snip*
[/quote]

FYI, putting raw pointers into a container like [font=courier new,courier,monospace]vector[/font] like that is a fantastic way to introduce memory leaks, double deletes, etc. Best use a smart pointer.

Share this post


Link to post
Share on other sites
CatmanFS    199
vector <int*> names; works, so then wouldn't you have a vector of structs or classes if the int* referred to the addresses in memory of different data structs?

Share this post


Link to post
Share on other sites
BitMaster    8651
[quote name='CatmanFS' timestamp='1355295597' post='5009726']
vector <int*> names; works, so then wouldn't you have a vector of structs or classes if the int* referred to the addresses in memory of different data structs?
[/quote]

For the reasons above having raw pointers to Rectanglee in the vector is generally not a good idea unless you have the experience to know what you are really doing (and if the OP is confused by a private copy constructor then he is definitely not there). Dropping all type safetype by forcing a cast from int* to Rectanglee* is much, much worse for absolutely no benefit. You lose all type safety and gain exactly nothing over the Rectanglee* implementation. You also have the option of walking well off into undefined behavior if you do not understand the casts between the involved pointers very well and depending on how your inheritance hierarchy looks. And you still have to allocate storage for the Rectanglee somewhere somehow for the pointers to point to.

Even setting those things aside, why would you store int* instead of Rectanglee*? If something like that has to be done (and I cannot imagine any reason out of the top of my head that is not done better by other methods), why not use at least void*?

That said, the obvious candidate to solve the issues is boost::shared_ptr. In C++11 either std::shared_ptr or std::unique_ptr would do the job. Of course if we had C++11 I would rather make the involved type movable and just stick them into containers normally (if possible).

Share this post


Link to post
Share on other sites
rnlf_in_space    1894
CatmanFS: Not quite sure, what you mean, but if you want to have a vector<int*>'s element point to instances of anything different than ints, you will have to use an explicit cast. This will work maybe, but is more or less one of the most evil things to do.

Also, as Cornstalks already said, using raw pointers instead of smart pointer creates a lot of potential problems later on.

Edit: you ninja, BitMaster Edited by rnlf

Share this post


Link to post
Share on other sites
black_darkness    280
[quote name='uglybdavis' timestamp='1355283813' post='5009685']
May i suggest (read the comments, i changed them:

[source lang="cpp"]int main() {
Point tl(100,100);
Simple_window win(tl,400,400,"My Window");
vector<Rectanglee*> rects; // Pointer Magic!

for (int i=0;i<8;i++){
int xa=(i*50)-1;
for(int j=0;j<8;j++){
int ya=(j*50)-1;
rects.push_back(new Point(i,j),50,50);
}
}
for(int i=0;i<64;i++){
win.attach(*(rects[i])); //Remember to de-reference
}
win.wait_for_button(); //wait until user clicks the next button to close window

// Clean up at some point!
for (unsigned int i = 0, size = rects.size(); i < size; ++i)
delete rects[i];
}[/source]
[/quote]

I read this article [url="http://www.cplusplus.com/doc/tutorial/pointers/"]http://www.cplusplus...orial/pointers/[/url] . And from what I understand making rects a pointer would make it incapable of using push_back which is the whole reason I am using a vector in the first place.

I suppose if I did something like this. It might work.

[CODE]
for (int i=0;i<8;i++){
int xa=(i*50)-1;
for(int j=0;j<8;j++){
int ya=(j*50)-1;
(*rects).push_back(new Point(i,j),50,50);
}
}
[/CODE] Edited by black_darkness

Share this post


Link to post
Share on other sites
SiCrane    11839
Don't make rects a pointer, make it a vector containing pointers. Preferably smart pointers, but regular pointers would also work. Either way it's legal to store both regular and smart pointers inside a vector, including using push_back().

Share this post


Link to post
Share on other sites
black_darkness    280
[quote name='SiCrane' timestamp='1355333123' post='5009892']
Don't make rects a pointer, make it a vector containing pointers. Preferably smart pointers, but regular pointers would also work. Either way it's legal to store both regular and smart pointers inside a vector, including using push_back().
[/quote]

So something like this. I have never used smart pointers. But I will look into it.

[CODE]
for (int i=0;i<8;i++){
int xa=(i*50)-1;
for(int j=0;j<8;j++){
int ya=(j*50)-1;
Rectanglee rect(Point(i,j),50,50);
rects.push_back(&rect);
}
}
[/CODE] Edited by black_darkness

Share this post


Link to post
Share on other sites
SiCrane    11839
You shouldn't store a pointer to an object on the stack in a vector. Use new to create your object and store that pointer instead.

Share this post


Link to post
Share on other sites
kunos    2254
[quote name='black_darkness' timestamp='1355333417' post='5009894']
[quote name='SiCrane' timestamp='1355333123' post='5009892']
Don't make rects a pointer, make it a vector containing pointers. Preferably smart pointers, but regular pointers would also work. Either way it's legal to store both regular and smart pointers inside a vector, including using push_back().
[/quote]

So something like this. I have never used smart pointers. But I will look into it.

[CODE]
for (int i=0;i<8;i++){
int xa=(i*50)-1;
for(int j=0;j<8;j++){
int ya=(j*50)-1;
Rectanglee rect(Point(i,j),50,50);
rects.push_back(&rect);
}
}
[/CODE]
[/quote]

that is a crash man. You cannot grab the address of a stack variable like that just as you cannot return a pointer to a variable declared into a function. Once it goes out of scope you'll be pointing at garbage memory.

rects.push_back(new Rectangle(bbla bla bla));

Share this post


Link to post
Share on other sites
black_darkness    280
[quote name='kunos' timestamp='1355333690' post='5009896']
[quote name='black_darkness' timestamp='1355333417' post='5009894']
[quote name='SiCrane' timestamp='1355333123' post='5009892']
Don't make rects a pointer, make it a vector containing pointers. Preferably smart pointers, but regular pointers would also work. Either way it's legal to store both regular and smart pointers inside a vector, including using push_back().
[/quote]

So something like this. I have never used smart pointers. But I will look into it.

[CODE]
for (int i=0;i<8;i++){
int xa=(i*50)-1;
for(int j=0;j<8;j++){
int ya=(j*50)-1;
Rectanglee rect(Point(i,j),50,50);
rects.push_back(&rect);
}
}
[/CODE]
[/quote]

that is a crash man. You cannot grab the address of a stack variable like that just as you cannot return a pointer to a variable declared into a function. Once it goes out of scope you'll be pointing at garbage memory.

rects.push_back(new Rectangle(bbla bla bla));
[/quote]


I tried that, but it says Rectanglee has no default constructor. I can see now that this problem is too advanced for me. I give up now. Thanks for the help.

Share this post


Link to post
Share on other sites
uglybdavis    1065
[b]@black_darkness[/b]

[source lang="cpp"]vector<SomeData>* pVector; // Pointer to a vector
pVector = new Vector<SomeData(); // Make new vector
pVector->push_back(something);
(*pVector).push_back(something);
delete pVector; // Delete the vectors

vector<SomeData*> Vector; // Vector storing pointers
Vector.push_back(new SoemData()); // Make new data
delete Vector[0]; // Delete the elements

vector<SomeData*>* pVector; // Vector pointer to pointers
pVector = new vector<SomeData*>(); // Make the data structure
pVector->push_back(new SomeData()); // Make new data
(*pVector).push_back(new SomeData()); // Just syntax difference
delete pVector[0]; // Delete elements first
delete pVector[1]; // We added two elements
delete pVector; // Delete the vector[/source]

Seems like you are confused about what a vector is / how memory is managed. Also seems that you don't fully understand what the stack / heap are. The beauty of C++ is that YOU are responsible for the memory. Forget about all the fancy smart pointer work, you use C++ in games for the speed, don't add overhead. Learn how to manage your memory. Seriously, learn memory management it's a very important skill.

Does the code sample above shed some light as to what's going on? Edited by uglybdavis

Share this post


Link to post
Share on other sites
kunos    2254
i think you forgot the "bbla bla bla" part [img]http://public.gamedev.net//public/style_emoticons/default/tongue.png[/img]

There you go:

vector<Rectangle*> rects;

rects.push_back(new Rectangle(Point(i,j),50,50));

This will work.. but I agree with you, it's a good thing for you to step back and get more familiar with C++ .

Share this post


Link to post
Share on other sites
black_darkness    280
[quote name='uglybdavis' timestamp='1355335965' post='5009911']
[b]@black_darkness[/b]

[source lang="cpp"]vector<SomeData>* pVector; // Pointer to a vector
pVector = new Vector<SomeData(); // Make new vector
pVector->push_back(something);
(*pVector).push_back(something);
delete pVector; // Delete the vectors

vector<SomeData*> Vector; // Vector storing pointers
Vector.push_back(new SoemData()); // Make new data
delete Vector[0]; // Delete the elements

vector<SomeData*>* pVector; // Vector pointer to pointers
pVector = new vector<SomeData*>(); // Make the data structure
pVector->push_back(new SomeData()); // Make new data
(*pVector).push_back(new SomeData()); // Just syntax difference
delete pVector[0]; // Delete elements first
delete pVector[1]; // We added two elements
delete pVector; // Delete the vector[/source]

Seems like you are confused about what a vector is / how memory is managed. Also seems that you don't fully understand what the stack / heap are. The beauty of C++ is that YOU are responsible for the memory. Forget about all the fancy smart pointer work, you use C++ in games for the speed, don't add overhead. Learn how to manage your memory. Seriously, learn memory management it's a very important skill.

Does the code sample above shed some light as to what's going on?
[/quote]

At this moment no. That probably has to do with a head-ache I have at the moment. I will come back to this thread and post when the head-ache goes away. Thanks for the help and excuse me for all the trouble.

Share this post


Link to post
Share on other sites
brx    720
There's absolutely no need to apologize.

Memory management in C++ is probably THE thing that a lot of people never get right and they just give up trying to learn C++ after a while. The [u]basics[/u] are not THAT difficult, though:

The value of a variable has a "lifetime". This lifetime ends with the end of the current scope. I.e. it ends when the next "}" is encountered. So this:

[CODE]
if (true) {
Rectanlge rect;
}
cout << rect.getX();
[/CODE]

will lead to a compiler error since rect is not valid anymore at the point where rect.getX() is called. In the moment the closing "}" is encountered the Rectangle object called "rect" is destroyed. [size=2](Keep reading, I know, it's not a great example, since this is a compiler error, and the problem at hand is a runtime problem, just trying to get a feeling for scoping here...)[/size]

So for your case this means:
[CODE]
for(int j=0;j<8;j++){
int ya=(j*50)-1;
Rectanglee rect(Point(i,j),50,50);
rects.push_back(&rect);
}
[/CODE]

every time the closing brace of the for-loop is hit, the object formerly know as "rect" is destroyed.

Now, your code saves the [u]address[/u] of that object inside the vector. Too bad... the object is dead when the for-loop is left... So that address is nothing but garbage.So in your vector there's a whole bunch (well 8...) addresses that used to be Rectangle objects but are not anymore...

So now, let's look at the "new" keyword. The rectangless we've created so far were on the stack. Objects on the stack behave like I just said. They are deleted once the variable goes out of scope. The second kind of memory is the "Heap". When you use "new" this will create a new object on the heap. That object is valid and available until
a) the program is terminated
or
b) you use "delete" to ... well... delete it.
So this:
[CODE]
for(int j=0;j<8;j++){
int ya=(j*50)-1;
Rectanglee* rect = new Rectangle(Point(i,j),50,50);
rects.push_back(rect);
}
[/CODE]
[size=2](note: used a temporary variable to make it clearer)[/size] will create 8 new Rectangle object on the HEAP that will be valid even after exiting the for-loop. Now... This may be a problem. Who will ever invalidate (i.e. delete those Rectangles)? They will be on the heap occupying memory until the program is exited (thanks to modern operating systems) or until you explicitly remove them using "delete".

So, if you want to get rid of those rectangles (all of them) this would work:
[CODE]
for(int j=0;j<8;j++){
delete rects[i];
}
rects.clear();
[/CODE]
This first deletes all the Rectangle objects that you've created on the heap and then clear the vector (because since we've just deleted all the rectangles, it contains nothing but a lot of addresses that formerly pointed to an object of type "Rectangle").

And this is exactly where smart pointers come in play... smart pointers help you to get rid of the burden to manually delete those objects that you've created on the heap, as they will automagically delete the object when there is no smart pointer left pointing at that object (it does not work all of the time [circular dependencies are a problem], but for a start it is pretty good)...


Hope this somewhat helps to get rid of the headache....

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this