Jump to content
  • Advertisement
Sign in to follow this  
charlando

class vector pointers help!

This topic is 4418 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Although the title is vague, have no fear, i will explain it now. I've been trying to get this class to work, i'm trying to make a vector pointer inside a class to point to a given vector outside the class. This is C++, it looks something like this.
class CTile
{
      private:
              vector<SDL_Rect> * Colpix;
      public:
             CTile( int x, int y, vector<SDL_Rect> BoxType );
};

CTile::CTile( int x, int y, vector<SDL_Rect> BoxType )
{
              for( int i = 0; i < BoxType.size(); i++ )
              {
                   *Colpix.push_back( &BoxType );
              }
}

...yea i'm using SDL too, the error for this is: 'push_back' has not been declaired request for member of non-aggregate type before '(' token Am i allowed to resize a pointer? I don't know what the problem is atall. - i am including all the right header files and i deleted some irrelevant stuff from the class to make it easier to see. Thanks for any help :)

Share this post


Link to post
Share on other sites
Advertisement
1° *a.b means *(a.b), and a->b means (*a).b

2° Use the initializer list, it's simpler
CTile::CTile( int x, int y, vector<SDL_Rect> & BoxType ) : Colpix(&BoxType) { }

3° Use a reference, it's probably simpler too
std::vector<...> & ColPix;
CTile::CTile( int x, int y, vector<SDL_Rect> & BoxType ) : Colpix(BoxType) { }


[Edited by - ToohrVyk on October 15, 2006 2:06:23 PM]

Share this post


Link to post
Share on other sites
add std:: in front of vector. Since its a pointer, you'll have to do Colpix->push_back. Your vector is also meant to hold SDL_Rects, rather than pointers to SDL_Rects. So currently your "vector" is just a pointer to a vector of SDL_Rects. What you really want is std::vector<SDL_Rect *> Colpix;. With that, it'll work fine. Also remove the * in front of Colpix in the CTile constructor.

Share this post


Link to post
Share on other sites
I think your compile error is due to the fact that you've not specified the namespace that vector is in. It should be std::vector, unless you've got using namespace std somewhere. The compiler is looking for a template called "vector" in the global namespace, and failing to find one. Read up on namespaces a bit.

However, aside from this there are other issues in your code, and I'm a bit confused as to what you're trying to do here.

For a start, in the example you've given, the value of ColPix will be undefined, and won't be pointing to a vector at the point where you call push_bacl. The behaviour will be undefined when you call *Colpix.push_back( &BoxType). ColPix won't be pointing to a vector unless you make it point to a vector. From looking at your code it looks like you've got the difference between a pointer to a vector, and a vector that holds pointers, mixed up. From looking at your code, I think you intended ColPix to be defined like this

vector<SDL_Rect*> ColPix; // Note that ColPix is a vector of pointers, not a pointer to a vector

Another thing to note, is that the BoxType parameter to the CTile constructor is being passed by value. This means that you're copying everything in the vector you pass in, every time you construct a CTile object. You should pass the vector in by constant reference, otherwise you're copying the entire vector and destroying the temporary copy every time you construct a CTile object.

CTile( int x, int y, const vector<SDL_Rect>& BoxType )

Another thing to note. You probably shouldn't be taking pointers to items in a vector like that. If you add more items than the vector has capacity for, at some point in the future, then vector will get resized and all your pointers will become invalid.

Looking at what you're trying to do. Personally I'd have BoxType be a class of its own, which stores a vector (or some other container) internally, rather than being a vector itself, and have CTile store a pointer or reference to a BoxType object, rather than passing around pointers to the vector. That way you can hide the implementation details from CTile, and (I assume) whatever class owns the BoxType vector. This will make your code a lot easier to change in future.

If however, all you want to do is make a vector pointer inside a class point to a vector outside the class. You need to change your code to this...




class CTile
{
private:

std::vector<SDL_Rect>* Colpix;
public:
CTile( int x, int y, std::vector<SDL_Rect>& BoxType );
};

CTile::CTile( int x, int y, vector<SDL_Rect>& BoxType )
: Colpix(&BoxType)
{

// This code is wrong
// you don't need to add the contents of
// BoxType to Colpix. Colpix is a pointer to the object BoxType
//
// What this code is doing is adding the contents of BoxType to BoxType
// even worse, it will probably result in an infinite loop
// because BoxType.size() will increase every time you go through
// the loop
/*
for( int i = 0; i < BoxType.size(); i++ )
{
*Colpix.push_back( &BoxType );
}
*/

}






[Edited by - Oxyacetylene on October 15, 2006 2:18:45 PM]

Share this post


Link to post
Share on other sites
0) We really can't work from "it looks something like this" and a retyping of errors. Although I discourage the use of copy and paste for developing code, it's essential for posting to the forums. Copy the *actual* code (indicating what's in which file) and the *actual* errors (including line numbers). (I can tell you haven't done so because every compiler vendor I've ever heard of can at least spell "declared" properly. [wink])

1) Think about what you're doing. When you make a pointer, it *does not* imply the existence of a pointed-at thing. You have to *point it at something*. Nowhere in your code does this occur. Instead, you are assuming something is there, and trying to push_back onto the pointed-at vector that isn't actually there.

2) Think about what you're doing. You have '*Colpix.push_back'; that tries to access the .push_back member function of the pointer (which is invalid; pointers don't have any members, the pointed-at things do), call the function, and dereference the result. The order of operations is wrong. As pointed out, you need the -> ("arrow") operator to make this work: Colpix->push_back.

3) Think about what you're doing. You create a pointer as a class member, but then instead of pointing it at some vector that exists somewhere else, you seem to want the object to have "its own" vector, and then *copy* the provided vector into the object's vector. That's ok, but it means you don't need or want a pointer-to-vector in the class. Just use a vector directly.

4) Think about what you're doing. You're copying a vector one element at a time, but the vector class already provides ways to make copies directly. The easiest would be via the copy constructor.

5) Prefer to pass objects by const reference.

6) Use typedefs to save typing and give descriptive names to your template container types.

7) Use initializer lists. In the provided code, the initializer list implicitly invokes the vector's copy constructor.


class CTile {
private:
typedef vector<SDL_Rect> box_list;
box_list Colpix; // No * here!

public:
CTile(int x, int y, const box_list& BoxType) : Colpix(BoxType) {}
};


That code causes the created CTile to have its own copy of the vector.

If you want the created CTile to have a pointer to a vector, and *point that pointer at the passed-in vector*, try this:


class CTile {
private:
typedef vector<SDL_Rect> box_list;
box_list* Colpix;

public:
CTile(int x, int y, const box_list& BoxType) : Colpix(&BoxType) {}
};


In this case, passing by reference is *required*; otherwise, the vector would be implicitly copied in order to call the function, and the object would set its pointer to point at the copy (which is *temporary*, just like a local variable) and that pointer would be garbage as soon as the constructor finished.

You have to be *very* careful when you do this kind of thing. Really, just use the first one.

I *could* show you how to make the class have a pointer to a vector, and set it up so that it points at a freshly created vector which is a copy of the input vector, and also show you how to make sure you don't get memory leaks as a result. But I won't, because it will never be the solution to your actual problem.

In general, there are *very* few reasons (in modern C++) to have a pointer as a data member of your own class or struct, because most of the good reasons are automatically wrapped up for you by various standard library widgets. (std::vector is itself a good example, actually.) Where the standard library fails, Boost fills in most of the gaps.

Share this post


Link to post
Share on other sites
Well thanks a lot for your help, it nearly worked after i changed 'vector<SDL_Rect> * Colpix;' to 'vector<SDL_Rect * > Colpix;'. I've come up with a better solution of using integers to refer to vectors - it should use less memory aswell. I've learned more about pointers and although i didn't succeed i thanks you all for your help.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!