Jump to content
  • Advertisement
Sign in to follow this  

is bad to use new/delete for huge class?

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

A while ago I posted a question here about the modeller that I had written. This modeller used only a single source file. Everything was global (including many, many flags and a few very large arrays - the global functions comprised about 6000 lines of code.) Then one day I learned that globals are "bad." So I tried to modulate the thing. I stuffed all the variables and functions into a Modeller class, and created an object of Modeller in my main loop. However, Modeller is such a huge class that there was an immediate stack overflow. So then I used "Modeller* modeller = new Modeller" and "delete modeller" and it all worked fine, with no stack overflow. So I'm just wondering - am I doing the right thing? Is there anything unsafe about this approach? Also, will my Modeller object correctly construct and destruct when I use new and delete? Thanks for your help! -synth_cat

Share this post


Link to post
Share on other sites
Advertisement
that is the correct approach. in fact, _large_ classes should only be allocated on the heap because otherwise you will certainly get stack overflow.

-me

Share this post


Link to post
Share on other sites
You could tweak the size of the stack.

At the very least, I would advise creating an autopointer to own the memory created. :)

Share this post


Link to post
Share on other sites
Quote:
Original post by synth_cat
Also, will my Modeller object correctly construct and destruct when I use new and delete?


Absolutely. new and delete invoke the constructor and destructor the same as if you created the object non-dynamically.

Share this post


Link to post
Share on other sites
Of course, if all you did was move your globals to a single class and give everything access to that class all you've done is made your globals more obnoxious to use without actually removing any of the issues with using global variables.

Share this post


Link to post
Share on other sites
Seconded.

Please show the Modeller class as it currently stands. "Big arrays" are a warning sign of a large static allocation being used where a dynamic allocation should be. You can safely stack-allocate an object that "owns" a lot of data if it does so via standard library containers etc., because only a small amount of bookkeeping goes on the stack, and the actual storage is dynamically allocated (and automatically managed by the object: when an object destructor is called, member destructors will subsequently be called, so you can avoid lots of work that way). In any case, passing the object around is likely a good idea. Failing that, give it a useful *interface*, rather than expecting client code to reach in and grab at a free-for-all of aggregated data.

Regardless of the "access pattern" you choose, if you require heap allocation, consider using std::auto_ptr() to ensure that deletion occurs when the object falls out of scope. (That's no help for static objects, admittedly.)

Share this post


Link to post
Share on other sites
Well, I guess if you really want to see it, I can post it. But I warn you, it's not pretty. This modeller is probably about as badly-written as it could be. You will probably be wondering why I didn't try and break this up into different classes, like a class for Direct3d and a class for point manipulations, or whatever. The answer is that it would be a lot of tedious work to do this, because my existing code structure is so arcane.

//here's some defines to give you the size of the big array members
#define POINTS_SIZE 40000
#define TEMP_TRIS_SIZE 1000
#define TEMP_POINTS_SIZE 3000

//here's the actual class definition -
class MapMaker
{

public:

LPDIRECT3D9 d3d;
LPDIRECT3DDEVICE9 d3ddev;
LPDIRECT3DVERTEXBUFFER9 vb_world;
LPDIRECT3DVERTEXBUFFER9 vb_text;
LPDIRECT3DVERTEXBUFFER9 vb_cells;
LPDIRECT3DVERTEXBUFFER9 vb_fx;
LPDIRECT3DVERTEXBUFFER9 vb_hud;
LPDIRECT3DVERTEXBUFFER9 vb_hudtex;
LPDIRECT3DVERTEXBUFFER9 vb_line;
LPDIRECT3DVERTEXBUFFER9 vb_hudline;
LPDIRECT3DTEXTURE9 t_text;
LPDIRECT3DTEXTURE9 t_tile;
LPDIRECT3DTEXTURE9 t_cells;
LPDIRECT3DTEXTURE9 t_fx;
VERTEXTEXT vb_text_temp[6*TEXT_LENGTH];

/////MAJOR ARRAYS AND BUFFERS
TRI tris[TRIS_SIZE];
SYNTH_POINT pnts[POINTS_SIZE];
MM_POLYGON polys[MAX_POLYGONS];
MM_CIRCLE circles[MAX_CIRCLES];
int current_circle; //indes to circles[]
int current_poly; //index to polys[]
int anchor_pnts[4];//part of tex-zoom

D3DXMATRIX m_view; //the global view transform matrix
D3DXMATRIX m_screenp;//

D3DXVECTOR3 v_viewup; //represents the vector that currently represents "up" onscreen
D3DXVECTOR3 v_viewright; //represents the vector the currently represents "right" onscreen
D3DXVECTOR2 v_screenpoint;

//The following two vectors are used for world transformations relative to which
//way the camera is facing; v_world_forward will move you in the direction your camera
//faces, for example
D3DXVECTOR3 v_world_forward; //represents moving "forward" in the world
D3DXVECTOR3 v_world_right; //represents moving "right" in the world

UINT text_active;

/////////////////////////////////GAME GLOBALS//////////////////////////
///////////////////////////////////////////////////////////////////////

//filename
char* filename;

//geometry creaton flags
float fps_scale;
int dimension1;
int dimension2;

//modes
GameMode game_mode;
SubMode mode;
int current_physics_type;

//x/y/z move switches
int tri_mov_x;
int tri_mov_y;
int tri_mov_z;

//indicates index of current triangle
int tri_selected;

//no-specific purpose debugger trackers
float test_float;
float test_float2;

int tri_last_clicked; //index to tris[]
int g_squaretex_a; //this is index to g_uv[]
int g_squaretex_b; //this is index to g_uv[]
int g_squaretex_c; //this is index to g_uv[]
int texpoint_selected; //this is index to g_uv[]
bool texpoint_active; //this is index to g_uv[]
float g_tile_x; //global uv coords from 0 to 1 which are used to draw the small
float g_tile_y; //texture square in the bottom left of the screen
float g_tex_x; //this indicates topleft of current tile
float g_tex_y; //this indicates topleft of current tile
float g_uv[4][2];
//mouse stuff - these variables must be refreshed manually
//within the main loop because only WinProc can be used to set them
//(in other words, there are global counterparts to all the following)
bool ms_active;
int ms_x, ms_y, ms_buttons;
MsState ms_left_state; //tracks left button
MsState ms_right_state;//tracks right button
int old_x, old_y, old_z; //track mouse-movements for transformations
//world cursor- this is a point on the y=0 plane which represents the position on this
//plane that the mouse cursor is pointing to. This point is only used for OMODE_CREATE_OBJECT
//under game mode GMODE_OBJECTS
D3DXVECTOR3 world_cursor;
bool xactive;
bool yactive;
bool zactive; //restrict transformations on axes
int currentx,currenty,currentz; //only applicable for point transformation
///world variables
float cam_mov_updown;
float cam_mov_leftright;
bool cam_moving;
float cam_x;
float cam_y;
float cam_z;
//note that cam_tgt is RELATIVE; the following are not absolute
float cam_tgt_x;
float cam_tgt_y;
float cam_tgt_z;

//timer stuff
int time_1;

//physics creation flags: regulate the properties of physics entities that get created
bool physics_entity_height;
bool physics_entity_use_existing_pnts;
bool physics_entity_outside_facing;
D3DXVECTOR3 physics_entity_creation_pos;
//entity_height indicates if this is a short or tall entity (short entities do not interact
//with flyer cells; that's the only difference
//entity_outside_facing indicates if entities keep cells outside or inside (true means that
//this entity keeps things outside


//functions
MapMaker();
void Run(int fps);
void CleanupD3D();
void ConfigurePoly(int idx);
void Draw();
void DrawPhysics();
bool Draw(int fps);
bool DrawMenu();
bool DrawPhysics(int fps);
float FindTriArea(D3DXVECTOR2 p1,D3DXVECTOR2 p2,D3DXVECTOR2 p3);
D3DXVECTOR2 GetPoint(D3DXVECTOR3 point);
void InitWorld(int width, int height);
bool InScreen(D3DXVECTOR2 point);
bool LoadFile();
void PrintIntD3D(int in_value, float p_x, float p_y);
void PrintStringD3D(char* string, float p_x, float p_y);
void RunBasicInput();
void RunCreateCircle();
void RunCreatePoly();
void RunD3D();
void RunEditPoly();
void RunGeometryEditInput();
void RunInputMenu();
void RunInputPhysics();
void RunPan();
void RunPntEdit();
void RunRotate();
void RunScale();
void RunSelectCircle();
void RunSelectPoly();
void RunSweep();
void RunTexEdit();
void RunTexMapEdit();
void RunTriEdit();
void SaveFile();
bool SetupD3D(HWND hwnd);
bool SetupTextures();
bool SetupVBs();

protected:

};




So there it is...

Now I have a few more questions:

Quote:

"Big arrays" are a warning sign of a large static allocation being used where a dynamic allocation should be.

Could you please explain what you mean? Is this a problem I currently have?

Another question:

This modeller of mine only ever needs to work on my own computer. However, I am concerned about my game, a separate project. This game contains a certain class with some rather large arrays in it. Together these arrays amount to 3-4 kB. So, what I'm wondering is: Is this big enough that I should worry about "big array" problems? Do I need to use new and delete as a safety measure? Is it possible that this class could overflow the stack on one of my users' computers?

One more question:
Quote:

Of course, if all you did was move your globals to a single class and give everything access to that class all you've done is made your globals more obnoxious to use without actually removing any of the issues with using global variables.

What sort of "issues" could eventually come up with this structure? Can you please explain?

Share this post


Link to post
Share on other sites
Well, if your program has only one class, most Object Oriented Principles will have little effect.

Share this post


Link to post
Share on other sites
Basically, the problem with globals is the same as the reason the are convenient : Any bit of code can modify them. So you can be happily coding away, and your code reaches for some global which, you think, "at this stage should be in thus-and-so a range", only to find that, oops, you'd forgotten about that bit of code over there that modifies it to be outside that range. (Just one example.) Even worse, you can have something like this :


// myGlobals.h

int someGlobalInt = 0; // Do not set to values greater than 16

(...)

// SomeOtherFile.cpp
someGlobalInt = 435; // Check documentation I wrote five weeks ago? Never!


Ok, it's a little unlikely for this to happen with ints, since you'd likely use an enum (right...?) but you see the point, I trust. And this sort of problem doesn't go away if you put your global in a class, it just means you have to write


myObnoxiousClass.badlyHiddenGlobal = 435; // Bwah-hah! I beat you with long variable names!


instead.

Now, it's true that this sort of thing is a lot less of a problem for one-man projects where only you are ever going to modify the code. So if that's the sort of thing you are doing, you're fine with globals. But if you want to share your code, be careful.

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.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!