Sign in to follow this  
Raeldor

Array of Pointers to Objects

Recommended Posts

Hi All, I know this has been covered 100 times, but I still couldn't find a solution to my problem. I have a dynamic array of pointers to Tile objects declared as... Tile **pTiles; I allocate the array using malloc, like... pTiles=(Tile **)malloc(nTilesAcross*nTilesDown*sizeof(Tile *)); and then initialize each tile using... for (int y=0; y < nTilesDown; y++) for (int x=0; x < nTilesAcross; x++) pTiles[x,y]=new Tile(nTileSize, x*nTileSize, y*nTileSize); Which seems to work fine. When I come to release however, I have problems. My release code looks like... // release tiles for (int y=0; y < nTilesDown; y++) for (int x=0; x < nTilesAcross; x++) { if (pTiles[x,y] != NULL) delete pTiles[x,y]; } // delete pointer array free(pTiles); It deletes the first tile at [0,0] fine, but when it trys to delete the second tile [1,0] I get an error because the destructor for the tile object is somehow pointing to the first tile [0,0] which has already been destroyed. What is going on here? Also, as I side issue why can I not declare the array of pointers using... pTiles=new (Tile *)[nTilesAcross, nTilesDown]; and then free it using... delete [] pTiles; Thanks all

Share this post


Link to post
Share on other sites
Let me be the first to say: Use std::vector or std::list. Ok.. now that's done.


OK, the problem with your code is, you can't index arrays like that. The reason is simple - the "," operator is "evaluate these two operands and return the second one.

So: "0,1" return 1, while "1,0" returns 0. "12,pie" returns pie.
This means that foo[0,1] will become foo[1].

So, index your arrays as foo[1][0] and so on.


Next, you can't actually alocate it like that - what you have there is a pointer to a pointer (**), you need to then alocate an array of pointers (for your columns), and then for each of those pointers, you then alocate your array of objects.

It would look something like:

// init your array of pointers
int** foo = new (int*)[10];

// for each pointer in the array, initialize an array of objects
for(size_t i = 0; i < 10; i++)
{
foo[i] = new int[10];
}

// then access your objects as so:
foo[0][0] = 1234;
dostuff(foo[2][8]);

// delete in reverse of how you created:
for(size_t i = 0; i < 10; i++)
{
delete[] foo[i];
}
delete[] foo;


And, of course, you should be able to fill in how to use maloc and free in place of new and delete (I recomend you use new and delete if you're in C++ - and I recomend you use C++ [smile]).



The alternative is to alocate the whole thing at once, and access parts of it manually:

// constants for width and height
const size_t W = 10, H = 10;

// The entire map:
int* foo = new int[W*H];

// Accessing your map, at location (w,y) (or you could use x,y or whatever)
int GetAtLocation(size_t w, size_t h)
{
return foo[w+h*W];
}


This method is probably more efficient. It uses less memory, and can probably be indexed much faster.

Share this post


Link to post
Share on other sites
Quote:
Original post by doho
I think pTiles[x,y] should be pTiles[x][y] or pTiles[y*nTilesAcross+x] depending on how you want to do it.


You are right... I am coming from C#! However, now...

pTiles=(Tile **)malloc(nTilesAcross*nTilesDown*sizeof(Tile *));
for (int y=0; y < nTilesDown; y++)
for (int x=0; x < nTilesAcross; x++)
pTiles[y*nTilesAcross+x]=new Tile(nTileSize, x*nTileSize, y*nTileSize);

works, but...

pTiles=(Tile **)malloc(nTilesAcross*nTilesDown*sizeof(Tile *));
for (int y=0; y < nTilesDown; y++)
for (int x=0; x < nTilesAcross; x++)
pTiles[x][y]=new Tile(nTileSize, x*nTileSize, y*nTileSize);

Does not!? I get a compile error saying...

f:\My Documents\Visual Studio Projects\test2\terrain.cpp(20): error C2679: binary '=' : no operator found which takes a right-hand operand of type 'Tile *' (or there is no acceptable conversion)

Can I not have two-dimensional arrays of pointers?

Thanks

Share this post


Link to post
Share on other sites
Quote:
Original post by Raeldor
Can I not have two-dimensional arrays of pointers?

No.

You can have a one dimentional array of pointers each with its own one dimentional array of pointers (which requires a two-stage alocation process - see my code). It will look just like a 2D array of pointers when you address it.

But yeah - the manual addressing method will probably be faster.

Share this post


Link to post
Share on other sites
Quote:
Original post by Andrew Russell
Quote:
Original post by Raeldor
Can I not have two-dimensional arrays of pointers?

No.

You can have a one dimentional array of pointers each with its own one dimentional array of pointers (which requires a two-stage alocation process - see my code). It will look just like a 2D array of pointers when you address it.

But yeah - the manual addressing method will probably be faster.


Thanks, I almost having it working now. Only thing that is not compiling at the moment is...

pTiles=new (Tile*)[nTilesAcross*nTilesDown];

I get the error...

f:\My Documents\Visual Studio Projects\test2\terrain.cpp(18): error C2143: syntax error : missing ';' before '['

Share this post


Link to post
Share on other sites
Quote:
Original post by anonuser
just to be picky its not good practice to mix delete, free, new , malloc.

And Shouldn't it be delete[] ?

and try
pTiles=new ((Tile*)[nTilesAcross*nTilesDown]);


I am trying to use just new and delete. I tried with the brackets and still got the error...

f:\My Documents\Visual Studio Projects\test2\terrain.cpp(18): error C2059: syntax error : '['

My new code reads...

pTiles=new ((Tile*)[nTilesAcross*nTilesDown]);

Share this post


Link to post
Share on other sites
You're going for this right?
pTiles[] ?
well you need to know sizes of port parts of the array.

pTiles=new Tile[nTilesAcross*nTilesDown];

will an array of Tiles that big.
assuming pTiles is defined as follows:
Tile **pTiles = new Tile[10][nTileAcross*nTilesDown];

will give you 10 arrays of arrays.

Get my drift?

when using new to generate an array that big, you do not do it like you would malloc.

new Type[size of array];

note this notaion is also used for pointers that are not arrays (won't discuss this further).

Share this post


Link to post
Share on other sites
Quote:
Original post by Raeldor
Thanks, I almost having it working now. Only thing that is not compiling at the moment is...

pTiles=new (Tile*)[nTilesAcross*nTilesDown];

I get the error...

f:\My Documents\Visual Studio Projects\test2\terrain.cpp(18): error C2143: syntax error : missing ';' before '['

Remove the brackets around Tile*. It looks like Andrew Russell is using gcc, which can cope with that syntax, but Visual C++ chokes on it. I'm not actually sure if it's valid syntax or not - looks too much like a call to placement new and this may be what confuses the compiler.

Enigma

Share this post


Link to post
Share on other sites
Quote:
Original post by Enigma
Quote:
Original post by Raeldor
Thanks, I almost having it working now. Only thing that is not compiling at the moment is...

pTiles=new (Tile*)[nTilesAcross*nTilesDown];

I get the error...

f:\My Documents\Visual Studio Projects\test2\terrain.cpp(18): error C2143: syntax error : missing ';' before '['

Remove the brackets around Tile*. It looks like Andrew Russell is using gcc, which can cope with that syntax, but Visual C++ chokes on it. I'm not actually sure if it's valid syntax or not - looks too much like a call to placement new and this may be what confuses the compiler.

Enigma


Awesome, that did the trick. I think I am good to go now.

Thanks all for your help!

Share this post


Link to post
Share on other sites
Quote:
Original post by Raeldor
pTiles=new (Tile*)[nTilesAcross*nTilesDown];


Quote:
Original post by Raeldor
pTiles=new ((Tile*)[nTilesAcross*nTilesDown]);


Quote:
Original post by anonuser
Tile **pTiles = new Tile[10][nTileAcross*nTilesDown];


These are all invalid syntax in standard C++, in the last one the types don't match.

@Raeldor if your making arrays of pointers just to get round the limitations of C-style dynamic arrays using operators new i.e. your type does not have a default constructor or you want to allocate memory first then incrementally construct/initialize elements instead of redundantly having all elements default constructed on first creation.

Doing it with an arrays of pointers is not an efficient method. What you really want is a dynamic array of the actual type but separate allocation/de-allocation & construction/destruction you can do this in C++ with operators new/delete but its very low-level and advance C++, potentially prone to error.

This is one of the reasons why we have std::vector it essentially takes care of all this for you among other things, guaranteed contiguousness of elements.

In both cases this can be done for multidimensional arrays too.

Share this post


Link to post
Share on other sites
Quote:

These are all invalid syntax in standard C++, in the last one the types don't match.

@Raeldor if your making arrays of pointers just to get round the limitations of C-style dynamic arrays using operators new i.e. your type does not have a default constructor or you want to allocate memory first then incrementally construct/initialize.

Doing it with an arrays of pointers is not an efficient method. What you really want is a dynamic array of the actual type but separate allocation/de-allocation & construction/destruction you can do this in C++ with operators new/delete but its very low-level and advance C++, potentially prone to error.

This is one of the reasons why we have std::vector it essentially takes care of all this for you among other things, guaranteed contiguousness of elements.

In both cases this can be done for multidimensional arrays too.


Just to recap, my code now looks like...

// create tiles
pTiles=new Tile*[nTilesAcross*nTilesDown];
for (int y=0; y < nTilesDown; y++)
for (int x=0; x < nTilesAcross; x++)
pTiles[y*nTilesAcross+x]=new Tile(nTileSize, x*nTileSize, y*nTileSize);

to initialize and...

// release tiles
for (int y=0; y < nTilesDown; y++)
for (int x=0; x < nTilesAcross; x++)
{
if (pTiles[y*nTilesAcross+x] != NULL)
delete pTiles[y*nTilesAcross+x];
}

// delete pointer array
delete [] pTiles;

Is this really bad c++? What additional benefit would std::vector give me considering I only want to index into these tiles using x/y values?

Thanks

Share this post


Link to post
Share on other sites
The code you have posted requires ((nTilesAcross × nTilesDown) + 1) memory allocations. One for the array and one for each Tile. Memory allocation is usually an expensive operation, although it doesn't usually matter too much for instances like this since this initialisation is likely to only be done once per level/game. Each access to a tile also requires two levels of indirection. One to index into the array and one to index from the pointer to the actual Tile. Using std::vector you could reduce this to just two memory allocations and reduce the levels of indirection to just one:
std::vector< Tile > tiles;
tiles.reserve(nTilesAcross * nTilesDown);
for (int y=0; y < nTilesDown; y++)
{
for (int x=0; x < nTilesAcross; x++)
{
tiles.push_back(Tile(nTileSize, x*nTileSize, y*nTileSize));
}
}
// example use
tiles[(y * nTilesAcross) + x].doSomething();




This also has the added benefit that the vector will automatically clean up it's memory when it leaves scope, so there is no need to delete([]) anything.

Enigma

Share this post


Link to post
Share on other sites
Quote:
Original post by Enigma
The code you have posted requires ((nTilesAcross × nTilesDown) + 1) memory allocations. One for the array and one for each Tile. Memory allocation is usually an expensive operation, although it doesn't usually matter too much for instances like this since this initialisation is likely to only be done once per level/game. Each access to a tile also requires two levels of indirection. One to index into the array and one to index from the pointer to the actual Tile. Using std::vector you could reduce this to just two memory allocations and reduce the levels of indirection to just one:
*** Source Snippet Removed ***
This also has the added benefit that the vector will automatically clean up it's memory when it leaves scope, so there is no need to delete([]) anything.

Enigma


Wow, that's pretty sweet looking code. I will definitely look into the vectors.

Thanks again!

Share this post


Link to post
Share on other sites
Just to add to the last comment, doing the arrays of pointers idiom to get round default construction increases the chance of memory fragmentation thus reducing efficency because you will not have real contiguousness of elements any more.

[Edited by - snk_kid on May 28, 2005 6:28:37 PM]

Share this post


Link to post
Share on other sites
Boost.MultiArray

Example:

#include "boost/multi_array.hpp"
#include <cassert>

int
main () {
// Create a 3D array...
typedef boost::multi_array<double, 3> array_type;
typedef array_type::index index;
// ... that is 3 x 4 x 2
array_type A(boost::extents[3][4][2]);

// Assign values to the elements
int values = 0;
for(index i = 0; i != 3; ++i)
for(index j = 0; j != 4; ++j)
for(index k = 0; k != 2; ++k)
A[i][j][k] = values++;

// Verify values
int verify = 0;
for(index i = 0; i != 3; ++i)
for(index j = 0; j != 4; ++j)
for(index k = 0; k != 2; ++k)
assert(A[i][j][k] == verify++);

return 0;
}

Share this post


Link to post
Share on other sites
Quote:
Original post by Enigma
It looks like Andrew Russell is using gcc, which can cope with that syntax, but Visual C++ chokes on it.

Actually, Andrew Russell was just typing code in the browser [wink]. Helping with plain-ol' syntax errors is kinda hard when you don't check your code.

Share this post


Link to post
Share on other sites
This is kind of going back to the very beginning, but when you delete an array of pointers, shouldn't you just delete the array itself? Like this:

Tile** tiles = whatever;
// do something
delete tiles;

Instead of deleting each one individually. This is irrelevant, I just was curious 'cause that's how I would have done it. Am I wrong?

Share this post


Link to post
Share on other sites
Quote:
Original post by evanofsky
This is kind of going back to the very beginning, but when you delete an array of pointers, shouldn't you just delete the array itself? Like this:

Tile** tiles = whatever;
// do something
delete tiles;

Instead of deleting each one individually. This is irrelevant, I just was curious 'cause that's how I would have done it. Am I wrong?


Yes your wrong, if you create an array of pointers with array new then invoke new for each individual pointer then you must invoke delete on each individual pointer and then array delete on the actual array. If you don't you leak memory or/and resources.

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