Jump to content
  • Advertisement

Archived

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

hellz

How far to take error checking?

This topic is 5274 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
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]

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

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!