• Advertisement
Sign in to follow this  

problems with new [] (c++)

This topic is 3897 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

Hi everyone. For the record, I'm using C++, the Code::Blocks IDE, and the Win32 API. Here's my problem. I've been developing a 2-D map editor for quite some time now, and I think new [] can speed up the app dramatically. I have 3 layers, and I declare each layer in a 2-D array like this...
#define MAX_WIDTH 500
#define MAX_HEIGHT 500

SMap LayerNum[MAX_WIDTH][MAX_HEIGHT]; // SMap is a structure I made containing tile/map information 

Anyway, that makes for a slow start to the program. Why would I need to initialize a 500x500 array if I only wanted a 32x32 map? It's a waste of memory if you ask me. So I decided to use new[] to allocate the width and height at runtime. So I did this...
SMap *LayerNum;

... 

LRESULT CALLBACK FileNewProc(){ // For the File - New dialog box
  ...

  MapWidth = GetDlgItemInt(...); // Needless to say, MapWidth is retrieved
  MapHeight = GetDlgItemInt(...);

  // Now I would initialize the layers according to map width/height
  LayerNum = new SMap[MapWidth][MapHeight];

  ...
}

But I come back with a compile error stating this... -MapHeight' cannot appear in a constant-expression To my knowledge, I thought you could use variables to initialize arrays. Maybe I don't fully understand new[] yet. Can anyone help me find a solution?

Share this post


Link to post
Share on other sites
Advertisement
you cannot new multi-dimensional arrays. you have to do this:


LayerNum = new SMap[MapWidth*MapHeight];



to access an x,y coordinate:


void get( int w, int h )
{
assert( w < MapWidth );
assert( h < MapHeight );
return SMap[ MapHeight*h + w ];
}



-me

Share this post


Link to post
Share on other sites
As Palidine said, you cannot use new to directly allocate multi-dimension arrays. There are a variety of solutions. He presented the (in my opinion) second-best option.

My question to you is, why are you just using a naked array of these SMap objects (and why is the name "Map" if they appear to actually be "Tiles" in the map?). I would suggest that you extend Palidine's suggestion by renaming SMap to Tile, and fully-encapsulating the array of Tile objects in a Map class. I would also advocate the use of std::vector for the array, instead of raw pointers, because it simplifies the cleanup. Like so:

#include "Tile.h" // defines Tile, what used to be SMap

class Map
{
public:
Map(int mapWidth,int mapHeight)
: width(mapWidth), // need to store width for GetTile()
tiles(mapWidth* mapHeight) // tiles has w(idth * height) Tile objects.
{
}

Tile& GetTile(int x,int y)
{
return (tiles[y * width + x]);
}

private:
int width;
std::vector<Tile> tiles;
};

This is an application of the principal of providing a "2D interface to a 1D" array, the same thing Palidine suggested but encapsulated slightly more. In general, this technique is preferable to dynamic multi-dimensional arrays (and sometimes even static ones) because it is much easier to construct, destroy, and manage. And you can access it with the same (x,y) kind of interface. Manually construction and tearing down n-d dynamically-allocated arrays requires lots of loops and loops of allocations; its prone to error and not cache-coherent.

If you like, you could replace GetTile() with an operator() overload (not operator[], though, as that can accept only a single parameter). You could (and should) also provide const-correct variations of the tile accessor.

Share this post


Link to post
Share on other sites
boost::multi_array.

And as mentioned, your names are really sucking.

'SMap'? I hope 'S' doesn't stand for 'struct'. You do realize that the fact that C++ has both 'class' and 'struct' keywords is only syntactic sugar, yes? There is no reason you would ever care, while writing the implementation code, whether a type was declared as a class or as a struct. (And even if you did, that's what 'go to definition' is for.)

'LayerNum'? That sounds like it should be either the number of layers, or a number indicating what layer we're on. The ambiguity already says it's a bad name; but both those interpretations are wrong; it's actually "a layer".

Speaking of which, where's your overall 3D thingy?

Share this post


Link to post
Share on other sites
Ok for you Zahlman, I don't know where you come off flaming me for what I name my variables. And as for LayerNum, I just did that so I wouldn't have to type out Layer1, Layer2, and Layer3. In the end, it is really my preference what I call everything in my code, is it not?

Thank you jpetrie and Palidine for your helpful input. I have contributed to both of your ratings.

Share this post


Link to post
Share on other sites
Zahlman is hardly flaming you. He's giving constructive criticism. The fact that he's being blunt about it doesn't warrent your accusation of flaming. His suggestions regarding naming are good one's and have been echoed by jpetrie.

Whether you choose to listen to their comments regarding naming or not is up to you - however both have pointed out alternatives to new[] namely std::vector and boost::multi_array either of which is a far better solution than manual memory allocation particularly in this case.

Share this post


Link to post
Share on other sites
I agree with the fact that std::vector and boost::multi_array are better than what I'm using now. Zahlman's upfront tone just caught me off guard. Regardless, this thread is getting off subject, so I'm just going to end it now.

Share this post


Link to post
Share on other sites
Quote:
Original post by GuitarPlayer0912
I agree with the fact that std::vector and boost::multi_array are better than what I'm using now. Zahlman's upfront tone just caught me off guard. Regardless, this thread is getting off subject, so I'm just going to end it now.


You shouldn't attempt to artificially end a thread, there may be more discussion and insight to be had.

And just for your information, Zahlman is one of *the* most helpful people on this board. I've seen him re-write a beginners full program (quite often) just to hlep them and teach them.

If you want to get help from anonymous internet forums, realise that without body language it can be difficult to correctly gauge the tone of the author. It is best to assume that any "tone" you get from a post is accidental, especially if the content of the post is helpful.

However, we programmers often use ridicule as a way of discouraging bad programming practices. Its not aimed at you, its an attempt to put you on the right road [grin].

Share this post


Link to post
Share on other sites
You asked for help on a public board. That's far from a flame, thicker skin may be required for further internet travels ;-)

Share this post


Link to post
Share on other sites
Not the safest or most flexible solution but one that will give you something which you can use in code in an identical manner to pre-defined multi-dimension arrays is this:


// create m by n tile multi array
Tile** tileArray = new Tile*[m];
tileArray[0] = new Tile[m*n];
for(int i = 1; i < m; ++i)
{
tileArray = tileArray[i-1] + n;
}

// use array just like pre-defined array, eg:
Tile& rTile = tileArray[2][5];

// delete array when done
delete []tileArray[0];
delete []tileArray;


Like has been said though, stl or boost is much better.

Share this post


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

  • Advertisement