Error Handling

[bar]

//Plays Sound
void TfrmMain::Play_Update_Alert(void)
{
String SoundFilePath;
try
{
try
{
SoundFilePath =
ExtractFilePath(Application->ExeName)+"BIRD1.wav";
SoundFilePath = AnsiReplaceText(SoundFilePath,"\\","\\\\");
PlaySound(SoundFilePath.c_str(),NULL,SND_FILENAME);
}
catch(Exception &E)
{
ShowMessage(E.Message);
}
}
__finally
{
delete &SoundFilePath;
}
}
----------------------------------
Hello all

here is a function that plays sound from the source folder of .exe
I want to know how standard is the error handling or i need any changes for
better coding.

Thanks
SA

[chenzero]

Hi,
one correction in the code,
it dosen't need to delete the local variable SoundFilePath
in the finally statement.
the variable need to be deleted only when created with "new"
keyword.
for more info, please refer to
http://www.cplusplus.com/doc/tutorial/dynamic.html

Post by bar
//Plays Sound
void TfrmMain::Play_Update_Alert(void)
{
String SoundFilePath;
try
{
try
{
SoundFilePath =
ExtractFilePath(Application->ExeName)+"BIRD1.wav";
SoundFilePath = AnsiReplaceText(SoundFilePath,"\\","\\\\");
PlaySound(SoundFilePath.c_str(),NULL,SND_FILENAME);
}
catch(Exception &E)
{
ShowMessage(E.Message);
}
}
__finally
{
delete &SoundFilePath;
}
}
----------------------------------
Hello all
here is a function that plays sound from the source folder of .exe
I want to know how standard is the error handling or i need any changes
for better coding.
Thanks
SA

[Thomas Maeder [TeamB]]

Post by chenzero
one correction in the code,
it dosen't need to delete the local variable SoundFilePath
in the finally statement.

Correction to the correction: :-)

It absolutely *must* not delete the address of SoundFilePath.

[Thomas Maeder [TeamB]]

Post by bar
//Plays Sound
void TfrmMain::Play_Update_Alert(void)
{
String SoundFilePath;
try
{
try
{
SoundFilePath =
ExtractFilePath(Application->ExeName)+"BIRD1.wav";
SoundFilePath = AnsiReplaceText(SoundFilePath,"\\","\\\\");
PlaySound(SoundFilePath.c_str(),NULL,SND_FILENAME);
}
catch(Exception &E)

Exceptions are best caught by reference to const.

Post by bar
{
ShowMessage(E.Message);
}
}
__finally
{
delete &SoundFilePath;

This is a serious bug. Remove the try..__finally scaffolding
altogether and move the definition of SoundFilePath to where you know
how to initialize it.

Post by bar
}
}
----------------------------------
here is a function that plays sound from the source folder of .exe
I want to know how standard is the error handling or i need any
changes for better coding.

In event-based applications (that includes basically all GUI
applications), user code is usually called back by some underlying
machinery (native API or application framework).

I tend to catch exceptions that occur while processing an event near
the boundary between user code and the code that calls it back. From
the code you posted, I can't tell whether this is the case.

What we can tell, however, is that the error handling performed in
this code is very generic. Chances are that it can be moved to a place
"higher up" where less is known about the specific context where this
exception may occur.

[Remy Lebeau (TeamB)]

Post by bar
SoundFilePath = AnsiReplaceText(SoundFilePath,"\\","\\\\");

You should not be doing that at all. It is completely unnecessary.

Post by bar
catch(Exception &E)

Always catch exceptions by const reference instead:

catch(const Exception &E)

Post by bar
delete &SoundFilePath;

DO NOT do that!!! You did not allocate the memory with 'new', so there is
nothing to free with 'delete'. That is a very bad crash waiting to happen
because you are trying to free memory that is on the stack, not the heap.
The String is on the stack, so it will be freed automatically when it goes
out of scope. You do not need to do anything yourself for that.

Post by bar
I want to know how standard is the error handling

It is not even close to being correct. The only exception that function
could ever throw is if the String values could not be allocated in memory,
and that would be vary rare. So the only real error would be from
PlaySound(), but it returns a BOOL value of FALSE instead of throwing an
exception if it can't play the sound.

Post by bar
or i need any changes for better coding.

Yes. Everything in that function is wrong. Use this instead:

void TfrmMain::Play_Update_Alert(void)
{
String SoundFilePath = ExtractFilePath(Application->ExeName) +
"BIRD1.wav";
if( !PlaySound(SoundFilePath.c_str(), NULL, SND_FILENAME) )
ShowMessage("Cannot play the sound:\n" + SoundFilePath);
}


Gambit