How far to take error checking?

Started by
12 comments, last by hellz 20 years, 4 months ago
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
Advertisement
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]
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
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.

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 occuredpublic:	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 descriptionpublic:	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 sameprivate:	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]
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..)
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++).
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!"
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.
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();

This topic is closed to new replies.

Advertisement