Sign in to follow this  
Nyocia

Singleton class

Recommended Posts

So recently ive created a Direct3D project for school, kinda small project tough, with a terrain where you can walk around in, that kinda project I use a singleton class, can anyone give me some positive and negative sides of using this classes
#pragma once
#include <Windows.h>
#include <vector>
#include "cDirect3D.h"
#include "cCamera.h"
#include "cTerrain.h"
#include "cXFile.h"
#include "userinput.h"
#include "defines.h"
#include "font.h"



// SINGLETON Class that holds all the info all the classes need to reach. =) ****-> Johan.
class DataBase
{
	public:
		static DataBase* getInstance();	// Returns the instance of the database that we have made, if we haven´t it create one ****-> Johan.
		~DataBase();					// Destructor, this cleans all the crap.

		
		IDirect3DVertexBuffer9 * g_pVB;		// vertex buffer
		IDirect3DIndexBuffer9 * g_pIB;		// index buffer
		IDirect3DTexture9 * g_pTexture[4];	// texture handle
		int g_nVertices;					// number of vertices
		int g_nIndices;						// number if indices
		float * g_height;					// heightmap
		float g_offsY;						// camera height over ground
		cTerrain *m_terrain;
		Direct3D* d3d;
		UserInput g_ui;	
		class Camera* g_cam;
		class cFont*  m_font[NUMFONTS];
		class cXFile* m_xFile;
		
	private:
		DataBase();						// Default Constructor, read more in the DataBase.cpp ****-> Johan.
		DataBase(const DataBase &) {}	// Constructor witch take a database as a variable ****-> Johan.
		static DataBase *m_DataBase;	// The database itself ****-> Johan.
};


////////////////////////////////



/*****     MADE BY    *********
*      JOHAN JOHNSSON         *
******************************/
#include "cDatabase.h"
#include "cCamera.h"
#include "defines.h"



// ----------------------------------------------------------------------------
// Initialize - Set the database pointer to zero ****-&gt; Johan
// ----------------------------------------------------------------------------
DataBase* DataBase::m_DataBase = 0;



// ----------------------------------------------------------------------------
// getInstance - return singleton instance ****-&gt; Johan
// ----------------------------------------------------------------------------
DataBase* DataBase::getInstance()
{
	if( !m_DataBase )					// First time caller?
		m_DataBase = new DataBase();

	return m_DataBase;					// Return it!
}


// ----------------------------------------------------------------------------
// DataBase Default Constructor - Sets the variables to the proper value ****-&gt;Johan
// ----------------------------------------------------------------------------
DataBase::DataBase()
{ 
	g_pVB = NULL;												// vertex buffer
	g_pIB = NULL;												// index buffer

	g_nVertices = 0;											// number of vertices
	g_nIndices = 0;												// number if indices

	g_height = NULL;											// heightmap

	g_offsY = 0;												// camera height over ground

	g_cam = new Camera(D3DXVECTOR3(0, 0, 0), 0, D3DX_PI * 1.15f, 0);		// the camera
	d3d = new Direct3D();
	m_xFile = new cXFile[NUMOBJECTS];
}



// ----------------------------------------------------------------------------
// ~DataBase default destructor - Cleans up the shit we have made ****-&gt; Johan
// ----------------------------------------------------------------------------
DataBase::~DataBase()
{
	g_pTexture[0]-&gt;Release();
	g_pTexture[1]-&gt;Release();
	g_pTexture[2]-&gt;Release();
	g_pTexture[3]-&gt;Release();


	if(g_pVB)
		g_pVB-&gt;Release();
		
	if(g_pIB)
		g_pIB-&gt;Release();
	
	delete g_cam;
	delete[] m_xFile;
	
	
	delete d3d;
}

[Edited by - e-Slave on October 19, 2006 7:44:30 AM]

Share this post


Link to post
Share on other sites
Please, when posting large chunks of code, use [source lang="cpp"][/source] tags. It will make your post far easier to read. I suggest you to edit your post above to reflect this.

1) it is possible to destroy your singleton by doing a simple "delete DataBase::getInstance();". This is very dangerous, and should not be allowed. This is even more dangerous since you make some very strong assumptions in your destructor (the textures still exists and are valid, the VB is either NULL or valid, and so on).

2) you have tons of public variables - you should encapsulate them, as anyone is now able to break the invariant of your class (the invariant being something like the "working state" - breaking it means that your class doesn't contain valid data anymore, and will not react correctly in some occasion).

3) You don't need a singleton for this task. Your singleton is a global variable in disguise, and it's not the goal of the pattern. A global variable should be as stateless as possible in order to achieve a correct behavior (reentrance, concurrency) - yours is not (it has many member variables). This is even worse since your data are public.

I agree that these are only negative comments - but singletons are very hard to use correctly, and it appears that you don't use it the right way (I often wonder if there is actually a right way to use them). I suggest you to give a read to Washu's journal. It contains great advices about singletons and refactoring code to avoid using singletons (or search for singletonisis on gamedev.net).

Regards,

Share this post


Link to post
Share on other sites
Quote:
Original post by Emmanuel Deloget
(or search for singletonisis on gamedev.net).

Searching for singletonitis might yield better results... [grin]

Share this post


Link to post
Share on other sites
I am not really sure what you are trying to accomplish with this "database".

I don't see a relationg with d3d,vertexbuffers or texturehandles to any database.

I'd take the thing back to the drawing board and think it a bit about separating responsabilities. Then you might consider whether you'll need singletons or not ("no" is correct answer most of the time).

Perhaps what you are looking for is something like:

- A renderer class - does all the rendering and contains all D3D specific. Renderer has a resource manager for textures (they are quite d3d specific). Also, renderer owns or provides drawable meshes.

- Camera class, in my opinion it isn't part of the render (since it doesn't have any renderer specific data). Camera class contains things such as camera position, matrix.

- Lots of other classes

Cheers !

[Edited by - Demus79 on October 19, 2006 10:46:04 AM]

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