Using Pointers and Vectors Together

[dandymcgee]

Okay, so I'm trying to load a map from a text file into a vector of tiles. Here's my current (and probably terribly wrong) attempt at doing so:

Level Class (Private)

Code: Select all

std::vector<Tile> *mapData;

Level::Load()

Code: Select all

mapData.push_back( new Tile(x, y, tileType) );

Tile Constructor

Code: Select all

Tile::Tile(int X, int Y, int Sprite);

When I try to compile I get:

request for member `push_back' in `((Level*)this)->Level::mapData', which is of non-class type `std::vector<Tile, std::allocator<Tile> >*'|

If I can't figure out what's wrong with this I'll probably try using an array as opposed to a vector as I have a working example of doing it that way. I figured it'd be more of a learning experience if I didn't just copy & paste though

Looking forward to constructive criticism

[MarauderIIC]

Code: Select all

std::vector<Tile> *mapData;
mapData.push_back( new Tile(x, y, tileType) );

Judging from what you're pushing back, you meant to write

Code: Select all

vector<Tile*> mapData

. A vector of pointers. You have a pointer to a vector up there. Also don't be afraid of using namespace std;, save you a lot of typing.

I think that your error is reflecting the fact that with the declaration you have, you should have written mapData->push_back.

[Falco Girgis]

Yeah, I think that you wanted a vector of pointers, not a pointer to a vector.

Pop quiz: vectors of references cannot exist. Now somebody tell me why, and I will... shower you with praise.

[Arce]

Not 100% sure, but I'd guess several reasons.

References must be bound to real objects at definition time hence you can
never dynamically allocate an array of references. Also, they cannot be rebound to anything else...ever

Also, references may or may not resolve to run-time entities hidden to the programmer. A reference is almost just another name for the same variable; creating a vector of references is like creating a vector of syntax. You cannot dynamically name variable at run-time, as 'variables' amount to memory storage during execution and 'variable names' are forgotten.

[ismetteren]

If you wanted a pointer to a vector, i am pretty sure you would have to access the push_back method whit the -> operator instead of .

[dandymcgee]

So I use . when I wanted to access an object's method and -> when I want to access a pointer to an object's method, right? Well it seems to be working perfectly now that I've changed

This:

Code: Select all

std::vector<Tile> *mapData;

To This:

Code: Select all

std::vector<Tile*> mapData;

And This (used to be a vector of ints):

Code: Select all

mapData[k + j * TILE_ROW]

To This:

Code: Select all

mapData[k + j * TILE_ROW]->GetSprite()

Thanks so much for your help everyone

[dandymcgee]

One more question regarding destructors.

Code: Select all

Level::~Level()
{
    for( int i = mapData.size(); i >= 0; i-- )
    {
        delete mapData[i];
    }
    mapData.clear();
    writeLog("Map Unloaded from memory\n");
}

How does this look? (I'm doing it backwards because I read somewhere it's better that way although I'm not really worried about order right now unless it matters?)

[M_D_K]

One more question regarding destructors.

Code: Select all

Level::~Level()
{
    for( int i = mapData.size(); i >= 0; i-- )
    {
        delete mapData[i];
    }
    mapData.clear();
    writeLog("Map Unloaded from memory\n");
}

How does this look? (I'm doing it backwards because I read somewhere it's better that way although I'm not really worried about order right now unless it matters?)

That looks fine. But really you only have to call mapData.clear() it calls the deconstructor(if it has one).

[ismetteren]

So I use . when I wanted to access an object's method and -> when I want to access a pointer to an object's method, right? Well it seems to be working perfectly now that I've changed

yes, and variables. The reason is that to access a thing in a pointer whit the . operator, you would have to do some thing whit some parenteses(i know it isent spelled corectly, i mean these guys: "( and )") and the * operator. I cant remember how it looks, but i shuold be pretty ugly. Therefore the -> operator was invented.

.:EDIT:.
you said: "a pointer to an object's method"
i dont know what you meant, but you are using it when you want to acces a method(or variable/field) of a object, if the object is a pointer <-- it could probably be described in a better way, but i am not that good at english.

[dandymcgee]

That looks fine. But really you only have to call mapData.clear() it calls the deconstructor(if it has one).

Well mapData is a vector so it doesn't have a deconstructor (I think that's what you meant?), only Level does.

access a method(or variable/field) of a object, if the object is a pointer

That's exactly what I was trying to say, nicely put :P

[M_D_K]

That looks fine. But really you only have to call mapData.clear() it calls the deconstructor(if it has one).

Well mapData is a vector so it doesn't have a deconstructor (I think that's what you meant?), only Level does.

I meant the Tiles deconstructor.

[bugmenot]

Code: Select all

Level::~Level()
{
    for( int i = mapData.size(); i >= 0; i-- )
    {
        delete mapData[i];
    }
    mapData.clear();
    writeLog("Map Unloaded from memory\n");
}

Should be:

Code: Select all

Level::~Level()
{
    for( int i = mapData.size()-1; i >= 0; i-- )
    {
        delete mapData[i];
    }
    mapData.clear();
    writeLog("Map Unloaded from memory\n");
}

Note the -1.

[dandymcgee]

I wasn't 100% sure about .size()'s return value. Thanks for that bugmenot

[MarauderIIC]

Code: Select all

Level::~Level()
{
    for( int i = mapData.size()-1; i >= 0; i-- )
    {
        delete mapData[i];
    }
    mapData.clear();
    writeLog("Map Unloaded from memory\n");
}

Can instead be

Code: Select all

Level::~Level()
{
    for (vector<Tile*>::iterator i = mapData.begin(); i != mapData.end(); ++i)
    {
        delete (*i);
    }
    mapData.clear();
    writeLog("Map unloaded from memory\n");
}

:) But good job in putting .size() in the initialization statement instead of as the stop condition, lots of new folks put it in the stop condition and then their for loop runs slower than necessary.

[bugmenot]

But good job in putting .size() in the initialization statement instead of as the stop condition, lots of new folks put it in the stop condition and then their for loop runs slower than necessary.

Arguably by a very tiny amount especially for a loop that small and assuming the compiler doesn't optimise it already whether you are counting from or to 0.

As a personal preference, I don't count to 0 as I find it counter intuitive going backwards unless I am in performance critical code.

On the subject of micro optimisations, if you are using iterators then the code should be:

Code: Select all

Level::~Level()
{
    vector<Tile*>::iterator endItr = mapData.end();
    for (vector<Tile*>::iterator i = mapData.begin(); i != endItr; ++i)
    {
        delete (*i);
    }
    //mapData.clear(); // Note: No need to call clear as we are destroying the object that owns mapData
    writeLog("Map unloaded from memory\n");
}

This is due mapData.end() returning the iterator by value so using it in condition check of the for loop, means you invoke the cost of the copy constructor being called as well as the function call cost (if the compiler doesn't optimise this out). Again, the performance gain is negligible unless you are in a performance loop.