Question about class constructors

Started by
19 comments, last by fpsgamer 15 years, 10 months ago
I read somewhere that calling other functions and doing calculations in class constructor is bad. Is it true? In my opinion its bad practice cause you cant give return if something goes wrong.
http://www.daily-it.com
Advertisement
It's not true - although, in C++, calling virtual functions may not work as expected because the vtable (an array of member function pointers used to implement virtual functions behind the scenes) may not be built at that time.

If something goes wrong in a constructor the correct way to signal this is by throwing an exception.

Put everything into a constructor that is necessary for the proper initialisation of that object; having a separate Init function, or many setAttrib functions, to circumvent putting code into a constructor is bad practice.
It all depends on your definition of bad. If you have declared an instance of a class in a global scope, the constructor will be called before main(). So if the constructor calls functions that require main() to do something first, things might go wrong.

If something does go wrong in a constructor, you could throw an exception.
Steve 'Sly' Williams  Monkey Wrangler  Krome Studios
turbo game development with Borland compilers
so this is not bad

MyClass::MyClass()
{
hWnd = CreateWindow(...);

if(!hWnd) throw ERROR;

}

MyClass *Build()
{

try{

MyClass *p = new MyClass();

}
catch(int iError)
{
//error handling
}

}

[Edited by - TiitHns on June 10, 2008 7:04:16 AM]
http://www.daily-it.com
It is preferred to throw exception classes derived from std::exception, which is in <stdexcept>. There are a number of other predefined exception classes derived from std::exception are included in <stdexcept>.
Just a little supplement: throwing from constructors is preferred, but in contrast you should never throw from a destructor. Also, as was said, don't call virtual functions from either a constructor or destructor (directly or indirectly).
can anyone point out how to make it better?


CEngine *Create(TCHAR *lpstrWindowTitle, UINT iWidth, UINT iHeight, UINT iColorBits, UINT iDisplayFrequency, BOOL bFullscreen)
{
CEngine *pCEngine = 0;

try
{
if(!iWidth || !iHeight) throw CE_ERROR_MISSING_WIDTH_HEIGHT;

pCEngine = new CEngine(lpstrWindowTitle,iWidth,iHeight,iColorBits,iDisplayFrequency,bFullscreen);

if(!pCEngine) throw CE_ERROR_CONTEXT_ALLOCATION_FAILED;
}
catch(int iError)
{
//windows messagebox
return NULL;
}
return pCEngine;
}
http://www.daily-it.com
You can start by decoupling your engine from window properties [wink]

There's that, and new throws on failure, it doesn't return NULL (so that if check in there is useless). Unless you use new (std::nothrow) which makes it return NULL on failure.
Quote:Original post by TiitHns
can anyone point out how to make it better?


// include the standard C-style assert macro#include <cassert>// import the standard namespaceusing namespace std;// extern the CEngine object, so it is globally accessibleextern CEngine g_Engine;// define the error codesstatic const int ERROR_NONE = 0;static const int ERROR_OUT_OF_MEMORY = -1;// return a signed integer rather than the object itself to give better error reportingint Create(TCHAR *lpstrWindowTitle, UINT iWidth, UINT iHeight, UINT iColorBits, UINT iDisplayFrequency, BOOL bFullscreen){	// runtime assert compiles out in release build, removing overhead	assert( iWidth > 0 && iHeight > 0 );	// attempt to allocate the engine object	g_Engine = new CEngine(lpstrWindowTitle,iWidth,iHeight,iColorBits,iDisplayFrequency,bFullscreen);	if( g_Engine == NULL )	{		// slim chance of running out of memory on a windows platform, but hey .. it pays to be thorough.		return ERROR_OUT_OF_MEMORY;	}	// no problem	return ERROR_NONE;}

Checking return values is just as robust but without the performance penalties associated with SEH. If you use SEH everywhere in your engine then you're probably going to suffer a pretty nasty performance penalty.

Try to keep code out of your constructors, the 'hidden' overheads of temporary object constructors and suchlike can incur a further performance penalty if you are careless. Often the programmer will use the constructor to initialise an object to some known state, often zeroing memory only to store new values in the memory he just zeroed out. All in all it can add up to be a killer waste of cpu cycles, when workarounds require only trivial code changes.

I hope this helps, and good luck!
so you reccomend

CEngine g_Engine = new CEngine(pass in creation parameters);

g_Engine->CreateContext(); //int return type for error checking

also why use static const int ERROR_NONE is it better to use #define ?
http://www.daily-it.com

This topic is closed to new replies.

Advertisement