is bad to use new/delete for huge class?

Started by
48 comments, last by synth_cat 17 years, 9 months ago
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
Greg Philbrick, Game Developercoming soon . . . Overhauled CellZenith
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
You could tweak the size of the stack.

At the very least, I would advise creating an autopointer to own the memory created. :)
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.
If anything it would bad if you didn't use new and delete for large classes.

Beginner in Game Development?  Read here. And read here.

 

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.
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.)
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?
Greg Philbrick, Game Developercoming soon . . . Overhauled CellZenith
Well, if your program has only one class, most Object Oriented Principles will have little effect.
[size="2"]I like the Walrus best.
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.
To win one hundred victories in one hundred battles is not the acme of skill. To subdue the enemy without fighting is the acme of skill.

This topic is closed to new replies.

Advertisement