Archived

This topic is now archived and is closed to further replies.

How far to take error checking?

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

Hey all. Finally had a bit of time to work towards some basic GDI drawing (busy year), but after designing a class to load and store an external bitmap resource, it''s brought up a question. As the topic says, just how far would you take error handling? Here''s sort of what I''m talking about: BitmapObj.h
// Definition of class, CBitmapObj


#ifndef BITMAPOBJ_H
#define BITMAPOBJ_H
#pragma once

#include <windows.h>
#include <string>

class CBitmapObj
{
public:
	CBitmapObj();       // Constructor.

	~CBitmapObj();      // Destructor.


	// Utility functions.

	BOOL LoadBMP(std::string path);  // Load BMP from external file.

	BOOL PrepareDC();                // Prepare the memory DC.

	BOOL SelectBMP();                // Select the BMP into the DC.

	BOOL SaveDimensions();           // Save the BMP attributes.


	// Getter/setter functions.

	HDC GetMemDC();
	int GetWidth();
	int GetHeight();

private:
	HDC m_hMemDC;         // Memory DC.

	HBITMAP m_hBitmap;    // Handle to the bitmap resource.

	int m_width;          // Bitmap''s width.

	int m_height;         // Bitmap''s height.

};

#endif
BitmapObj.cpp
// Implementation of class, CBitmapObj.


#include "bitmapobj.h"

CBitmapObj::CBitmapObj()
{
	m_hMemDC = NULL;
	m_hBitmap = NULL;
	m_width = m_height = 0;
}

CBitmapObj::~CBitmapObj()
{
	m_hBitmap = NULL;

	if (m_hMemDC)
		DeleteDC(m_hMemDC);
}

BOOL CBitmapObj::LoadBMP(std::string path)
{
	m_hBitmap = reinterpret_cast< HBITMAP > (LoadImage(
		NULL, path.c_str(), IMAGE_BITMAP,
		0, 0, LR_LOADFROMFILE
	));

	return (m_hBitmap == NULL) ? FALSE : TRUE;
}

BOOL CBitmapObj::PrepareDC()
{
	m_hMemDC = CreateCompatibleDC(NULL);

	return (m_hMemDC == NULL) ? FALSE : TRUE;
}

BOOL CBitmapObj::SelectBMP()
{
	// No BMP handle saved, so simply return.

	if (m_hBitmap == NULL)
		return FALSE;

	return (SelectObject(m_hMemDC, m_hBitmap) == NULL) ? FALSE : TRUE;
}

BOOL CBitmapObj::SaveDimensions()
{
	// No BMP handle saved, so simply return.

	if (m_hBitmap == NULL)
		return FALSE;

	BITMAP bmpInfo; // Structure to hold BMP attributes.

	
	if (GetObject(m_hBitmap, sizeof(BITMAP), &bmpInfo) == NULL)
		return FALSE;

	m_width = bmpInfo.bmWidth;
	m_height = bmpInfo.bmHeight;
	
	return TRUE;
}

HDC CBitmapObj::GetMemDC() { return m_hMemDC; }
int CBitmapObj::GetWidth() { return m_width; }
int CBitmapObj::GetHeight() { return m_height; }
Apologies for the slight lack of commenting, only just finished that. Anyway, here''s an example of it in use: Extract from WinMain()
	CBitmapObj bmp;
	HDC hdc = GetDC(gameWin.m_hwnd);

	if (bmp.LoadBMP("Images/red.bmp") == NULL)
		MessageBox(NULL, "LoadBMP", "LoadBMP", MB_OK);

	if (bmp.PrepareDC() == NULL)
		MessageBox(NULL, "PrepareDC", "PrepareDC", MB_OK);

	if (bmp.SelectBMP() == NULL)
		MessageBox(NULL, "SelectBMP", "SelectBMP", MB_OK);

	if (bmp.SaveDimensions() == NULL)
		MessageBox(NULL, "SaveDimensions", "SaveDimensions", MB_OK);
	
	if (BitBlt(
		hdc, 0, 0, bmp.GetWidth(), bmp.GetHeight(),
		bmp.GetMemDC(), 0, 0, SRCCOPY
	) == NULL)
		MessageBox(NULL, "BitBlt", "BitBlt", MB_OK);

	ReleaseDC(gameWin.m_hwnd, hdc);
As you can see, I''ve added several calls to MessageBox(), just to illustrate my point. After seeing that little lot, do you think that would be overly cautious (imagining that calls to MessageBox() wouldn''t be the way any errors would be handled - something like a log file instead)? Thanks for any replies. -hellz

Share this post


Link to post
Share on other sites
Mess around with exception handling and see if you like it; I find that it cleans up my code an awful lot, but it took a while for me to convert. I still check function return values too, in case you're wondering, but only for things that wouldn't fail (i.e. checking if a file is open shouldn't throw an exception, but would return a bad value.)

edit: on a side note, i was reading about memory management and heard that you shouldn't check the return values for operator new() anymore; the reasoning is that if it fails, then the computer is so low on memory that it will crash anyway. Unless you're allocating an insane amount of memory, of course.

[edited by - psykr on December 16, 2003 6:07:16 PM]

Share this post


Link to post
Share on other sites
Thanks for the reply mate. That''s the sort of thing I was talking about. Funny you should mention about the new operator; someone said something similar to me a few days ago, but I''d not had time to check it out, so thanks for reminding me.

I guess in some ways, error handling is pretty much task specific. I mean, I''d imagine that if Windows had a problem returning a handle to a memory device context, that it''d be unlikely to be able to load a bitmap anyway. On the other hand, it''s easier to think in terms of handling possible errors with situations such as when opening files, because the error is more common and easy to get to grips with.

I should probably check out some code from a reputable engine, or something similar, just to see how other people tackle the problems I''ve been toying with in my mind for a while now.

Thanks again, for the reply.

-hellz

Share this post


Link to post
Share on other sites
I think exceptions should only be used if the error is recoverable.

For example, let''s say that your loadBMP fails. What are you going to do... play the game without the bitmap? Is that really acceptable? If you can''t play without the bitmap, then it''s easiest to just display an error and stop the game.

Share this post


Link to post
Share on other sites
I just finished my error handling class today, it does alright. I'm going to post the source January 1 if you want it. It's nothing major, but it's pretty nice.

Here's the header for it:

//::|ññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññ|::

//::|ññ ---------------------------------------------------------------------------------- ññ|::

//::|ññ| ~ A R I E L P R O D U C T I O N S ~ |ññ|::

//::|ññ| ~ ALL RIGHTS RESERVED ~ |ññ|::

//::|ññ ---------------------------------------------------------------------------------- ññ|::

//::|ññ| ~ PROGRAM DATA ~ /-----/ |ññ|::

//::|ññ| /-----/ | |ññ|::

//::|ññ| PROGRAMMER:: James Dougherty | | | | |ññ|::

//::|ññ| COPYRIGHT :: ©2003 Ariel Productions | | | | |ññ|::

//::|ññ| TYPE :: Utility | /---|-/ |ññ|::

//::|ññ| /-----/ |ññ|::

//::|ññ ---------------------------------------------------------------------------------- ññ|::

//::|ññ| ~ SOURCE DATA ~ |ññ|::

//::|ññ| |ññ|::

//::|ññ| FILENAME :: XErrorSystem.h |ññ|::

//::|ññ| VERSION :: 1.5 |ññ|::

//::|ññ| |ññ|::

//::|ññ| HISTORY DATA |ññ|::

//::|ññ| | |ññ|::

//::|ññ| |-10.03.03 (JD) |ññ|::

//::|ññ| |-Initial Build |ññ|::

//::|ññ| | |ññ|::

//::|ññ| |-12.09.03 (JD) |ññ|::

//::|ññ| |-Fixed clipboard failure |ññ|::

//::|ññ| | |ññ|::

//::|ññ| |-12.16.03 (JD) |ññ|::

//::|ññ| |-Added a new e-mail feature |ññ|::

//::|ññ| | |ññ|::

//::|ññ| |ññ|::

//::|ññ| BUGS |ññ|::

//::|ññ| | |ññ|::

//::|ññ| |-N/A |ññ|::

//::|ññ| | |ññ|::

//::|ññ| |ññ|::

//::|ññ ---------------------------------------------------------------------------------- ññ|::

//::|ññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññññ|::


#ifndef XERRORSYSTEM_H
#define XERRORSYSTEM_H
#include "XDefinition.h"

//--

//-- This will only enable the e-mail feature in the release build. This will save the

//-- programmer a lot of e-mails during the debug/build phase.

//--

#define ONLY_ENABLE_EMAIL_IN_RELEASE_BUILD 1

//--

//-- SMTP server address

//--

#define SMTP_SERVER_ADDRESS ":::::::::[NOBODY NEEDS THAT]::::::::::.net"

//--

//-- ::NOTE::

//-- The e-mail was designed for internal business use. If an error occurs in the program

//-- an e-mail can be sent instead of the associates giving you a call. This is a good idea

//-- if you have a massive amount of programs to deal with. Where I work at now, we have

//-- around 1,200+ programs that we have to deal with. Of course it's not in C++, or we would

//-- have 1 :) (Don't you love publisher reports and database programming!)

//--

#define SYSTEMS_PROGRAMMER_EMAIL_ADDRESS "arielproductions@zoominternet.net"

//--

//-- This will be the subject on the e-mail when it is sent.

//--

//-- ::NOTE::

//-- Change %Put Program Name Here% to the name of the current project you're working on.

//--

//-- ::NOTE::

//-- Notice there is no "]" at the end of the subject. This is because the error system

//-- will put the computers name at the end of the subject so you know what computer it

//-- came from. This is helpful when there are 10's to 100's of computers on your network.

//-- It becomes usfull because you can find out who was running the computer at the time

//-- the error occured so you can talk to them personally to find out what they were doing

//-- to cause the error.

//--

#define EMAIL_SUBJECT "[ERROR IN PROGRAM %Put Program Name Here% ON SYSTEM "

class XERROR
{
public: //--Error Results


enum XRESULT
{
XNOERROR = 0x00, //--No errors occured

XFALSE = 0x01, //--Process failed, but can continue (Warning)

XFATAL = 0x02, //--Fatal error

XNOMEMORY = 0x03, //--Not enough memory available to finish the process

XNOTIMPLEMENTED = 0x04, //--Currently not implemented

XFILENOTFOUND = 0x05 //--File does not exists

};

public: //--Error Actions


enum XERRORACTION
{
XEA_NONE = (1<<0), //--Perform no action

XEA_CLPBRD = (1<<1), //--Copy error message to the clipboard

XEA_MSGBOX = (1<<2), //--Display a message box

XEA_REPORT = (1<<3), //--Save to an error report

XEA_DEBUG = (1<<4), //--Print out a debug string

XEA_BREAK = (1<<5), //--Break into the debugger

XEA_EXIT = (1<<6), //--Exit the application

XEA_EMAIL = (1<<7), //--Send an e-mail to the systems programmer

XEA_FATAL = XEA_MSGBOX | XEA_REPORT | XEA_EMAIL | XEA_EXIT,
XEA_COMMON = XEA_CLPBRD | XEA_MSGBOX | XEA_REPORT | XEA_EMAIL,
XEA_FULL = XEA_COMMON | XEA_DEBUG | XEA_BREAK | XEA_EXIT
};

public:
XERROR();
XERROR(Xint);
XERROR(XRESULT);
XERROR(const XERROR&);

public:
Xbool Failed(Xvoid); //--See if the current error failed

Xbool Failed(const XERROR&); //--See if the query error failed

Xbool Succeeded(Xvoid); //--See if the current error succeeded

Xbool Succeeded(const XERROR&); //--See if the query error succeeded

Xvoid ProcessError(Xstring ErrorMessage); //--Process the error if one occured

Xvoid ProcessWarning(Xstring WarningMessage); //--Process the warning if one occured


public:
Xint GetFlags(Xvoid); //--Get the current error action flags

Xvoid SetFlags(Xint Flags); //--Set the error action flags

Xvoid SetResult(XRESULT Result); //--Set the current error result flag

Xuint GetErrorID(Xvoid); //--Get the current error ID

Xuint GetErrorID(const XERROR&); //--Get the query error ID

Xstring GetErrorDescription(Xvoid); //--Get the current error description

Xstring GetErrorDescription(const XERROR&); //--Get the query error description


public:
XERROR& operator= (const XERROR& Error); //--Copy an error

Xbool operator==(const XERROR& Error) const; //--See if two errors are the same

Xbool operator==(const XRESULT Result) const; //--See if two error results are the same

Xbool operator!=(const XERROR& Error) const; //--See if two errors are not the same

Xbool operator!=(const XRESULT Result) const; //--See if two error results are not the same


private:
Xint m_Flags; //--Holds the current error action flags

XRESULT m_CurrentResult; //--Holds the current error result flag

};

//------------------------------------------------------------------------------------------------

//--

//-- Error Handling Macros

//-- ---------------------

//--

//-- POSTERROR(*)

//-- ~INTERNAL::PRIVATE~ Called by XERROR()

//--

//-- POSTWARNING(*)

//-- ~INTERNAL::PRIVATE~ Called by XWARNING()

//--

//-- TRAPERROR(*)

//-- If CONDITION is 'true', it wall call POSTERROR() with the ERRORMESSAGE

//-- and FLAGS as the parameters.

//--

//-- TRAPWARNING(*)

//-- If CONDITION is 'true', it wall call POSTWARNING() with the ERRORMESSAGE

//-- The XEA_EXIT flag is disabled, can't exit on a warning.

//--

//-- ~ NOTE ~

//-- Flags are XERRORACTION flags.

//--

//------------------------------------------------------------------------------------------------

extern Xvoid POSTERROR(Xstring File, Xint Line, Xstring ErrorMessage, Xint Flags);
extern Xvoid POSTWARNING(Xstring File, Xint Line, Xstring WarningMessage, Xint Flags);

#define TRAPERROR(CONDITION, ERRORMESSAGE, FLAGS) { if((Xint)(CONDITION)) { POSTERROR(__FILE__, __LINE__, (Xstring)ERRORMESSAGE, (Xint)(FLAGS)); } }

#define TRAPWARNING(CONDITION, WARNINGMESSAGE, FLAGS) { if((Xint)(CONDITION)) { POSTWARNING(__FILE__, __LINE__, (Xstring)WARNINGMESSAGE, (Xint)(FLAGS)); } } }
#endif //--XERRORSYSTEM_H



EDIT:
Bad formatting...
It put the macros on 1 line!?!

-UltimaX-

"You wished for a white christmas... Now go shovel your wishes!"

[edited by - UltimaX on December 16, 2003 10:04:33 PM]

Share this post


Link to post
Share on other sites
Nekhmet: I may have misunderstood you, but I believe you''re saying the exact opposite of what I''m saying. I think that if you have an error that is recoverable (if that bitmap is just an extra feature), then you should return an error instead of throwing an exception. This way, you don''t have to pass the error up levels and levels of functions, instead you can just wait for the appropriate handler, and prevent wasting precious coding time typing those extra function checking routines. (I''ve forgotten the other advantages at the moment..)

Share this post


Link to post
Share on other sites
quote:
...heard that you shouldn''t check the return values for operator new() anymore...

A better reason is that the standard says that new throws std::bad_alloc, rather than returning 0 on failure. Checking for 0 only became popular because of compilers that do not directly conform to the standard in that area (like VC++).

Share this post


Link to post
Share on other sites
Nekhmet: Not only what psykr said, but how many games do you see shipped with the actual bitmap files. Ususally they use some resource file system anyway. So if your missing 1 image you have a lot more serious problems than just 1 image anyway.

-UltimaX-

"You wished for a white christmas... Now go shovel your wishes!"

Share this post


Link to post
Share on other sites
I think what the OP was talking about was the cost of doing all this checking, and is it going to hurt. The answer in short, yes, but not as much as you think. Put all the error checking you want in initalization, but putting TOO much into the main game loop may add a moderate performance hit. One way to avoid this, is to have specific #ifdef debug statements lieing throughout your code, and have it ignore the code when you are not in debug mode. This is tedious, but if you want the fastest code, its a necessity.

Share this post


Link to post
Share on other sites
quote:
Original post by merlin9x9
quote:
...heard that you shouldn''t check the return values for operator new() anymore...

A better reason is that the standard says that new throws std::bad_alloc , rather than returning 0 on failure. Checking for 0 only became popular because of compilers that do not directly conform to the standard in that area (like VC++).


Yes. If someone still want to check for 0 though it''s easy to disble the exception by using std::nothrow as in:

FOO* foo = new (std::nothrow) FOO();

Share this post


Link to post
Share on other sites
Thanks for the replies guys. Got quite a lot to go on there, and yes PaulCesar, that''s pretty much where I was heading with my question.

I''m thinking about downloading a 3D engine to have a look at; not necessarily for the 3D side of things, but more for the error handling at this point.

There were actually two reasons my question came up:

1. From working on the class I pasted into my original post.
2. Seeing how errors are returned from Unreal Tournament by their calling function(s), as in:

Exception at: MainLoop->GameEngine->Botpack->Warshell->etc.

Some of the error messages I''ve seen from UT have been absolutely huge (message boxes literally taking up an entire screen on 800x600).

Anyway, thanks again. I''ll have a nose around some engine code in a day or so.

-hellz

Share this post


Link to post
Share on other sites
UT uses the exception mechanism to build those error messages. They have 2 macros, guard and unguard:


SomeObject::SomeFunc()
{
guard(SomeObject::SomeFunc)
// code

unguard;
}


The guard macro starts the try block, but also stores the name of the function in a local static variable (and possible some other stuff). The unguard macro closes the try and implements the catch block. When an exception is caught, the function name in the local var is added to a list somewhere and the exception is rethrown. The function names in the list are then used to build a stack trace.

Additionally, two other versions of the macros, guardslow and unguardslow, are used in performance critical areas. These macros can be turned on for testing/debugging purposes, but are turned off in the release version.

The Torque engine takes a somewhat different approach, using a custom assert system. This is used most often to verify function params, but also to catch things like ''divide by zero'' and ''array out of bounds''. There are different macros for fatal and warning conditions, and also macros for release build and those that are turned on/off for testing and debugging. This sort of system can also be implemented in C.

Error checking can take away cycles, but it is important nonetheless. People who knock Java & C# often point at the fact that they are not compiled to native code as a reason for performance issues, when in fact it''s mostly because of all of the error checking going on behind the scenes. Those languages verify that every array access is in bounds, that every divide is safe, and things other than error checking. C and C++ allow you to be as stringent or as loose about error checking as you''d like, which is a great thing when performance is important.

I think a simple rule to follow is, write your code to check for errors everywhere the code can hurt you (verifying function params, return values, array indices where they may go out of bounds, etc...). Remove the checks that hurt performance only after profiling, but see if there are other ways of making the same check without the performance penalty (verifying a pointer once before calling a method multiple times, rather than in the method itself, for example). Also, if the game is moddable, it may be a good idea to look for optimizations elsewhere and leave the error checking code in if it''s something that can be unpredictable at runtime. Macros can greatly help in this, so they can be turned on and off when needed.

Share this post


Link to post
Share on other sites
quote:
Original post by Nekhmet
I think exceptions should only be used if the error is recoverable.

For example, let''s say that your loadBMP fails. What are you going to do... play the game without the bitmap? Is that really acceptable? If you can''t play without the bitmap, then it''s easiest to just display an error and stop the game.




But you can do that with an exception. Just catch fatal exceptions at the top level, cleanup, and bail.


--
Dave Mikesell Software & Consulting

Share this post


Link to post
Share on other sites