Do you think this is a bad idea or not( C++)
-
void SomeFunction()
{
try
{
//Do all kind of stuff
}
catch(_com_error& e)
{
MessageBox(GetErrorDescription(e));
}
catch(...)
{
MessageBox("Unexpected error. Please contact our tech support");
}
}What modification will you suggest?
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
Replace MessageBox with a TRACE perhaps?
Regards, Nish
Nish’s thoughts on MFC, C++/CLI and .NET (my blog)
Currently working on C++/CLI in Action for Manning Publications. Also visit the Ultimate Toolbox blog -
void SomeFunction()
{
try
{
//Do all kind of stuff
}
catch(_com_error& e)
{
MessageBox(GetErrorDescription(e));
}
catch(...)
{
MessageBox("Unexpected error. Please contact our tech support");
}
}What modification will you suggest?
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
Unless the exceptions are "expected" and have proper "handlers", I would throw the exception again to cause the app to fail or allow some external rountine to handle the exception. Throwing the exception again allows all the callers to free memory, locks, files, etc. as the application gracefully crashes. Without it, you have no clue what your operating state is. - Ian
-
void SomeFunction()
{
try
{
//Do all kind of stuff
}
catch(_com_error& e)
{
MessageBox(GetErrorDescription(e));
}
catch(...)
{
MessageBox("Unexpected error. Please contact our tech support");
}
}What modification will you suggest?
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
-
void SomeFunction()
{
try
{
//Do all kind of stuff
}
catch(_com_error& e)
{
MessageBox(GetErrorDescription(e));
}
catch(...)
{
MessageBox("Unexpected error. Please contact our tech support");
}
}What modification will you suggest?
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
In our code, if we do a general catch(...), we call a common unhandled exception handler that:
- logs the class name & method name that detected the problem
- displays a message box to the user that an unrecoverable error has occurred
- attempts a graceful shutdown of the application
We never try to continue because we have no idea how the application has been compromised. And should another unhandled exception occur while we are trying to clean up, we just give up and die immediately. [ Hmm... 'Unhandled Exception Handler' -- that just might qualify as an oxymoron! ]
Be clear about the difference between your role as a programmer and as a tester. The tester in you must be suspicious, uncompromising, hostile, and compulsively obsessed with destroying, utterly destroying, the programmer's software. ----- Boris Beizer
-
Unless the exceptions are "expected" and have proper "handlers", I would throw the exception again to cause the app to fail or allow some external rountine to handle the exception. Throwing the exception again allows all the callers to free memory, locks, files, etc. as the application gracefully crashes. Without it, you have no clue what your operating state is. - Ian
Mr Code Project wrote:
, I would throw the exception again to cause the app to fail or allow some external rountine to handle the exception.
What would be the point in catching the exception there if you are just gonna re-throw it? The exception would find its way up to the calling method regardless and if it has a try/catch block it would be caught by it. There is no point in catching exceptions deep in the core of your code if you are not going to do anything but re-throw it.
=====Brain melting code=====
static int Sqrt(int x){ if (x<0) throw new ArgumentOutOfRangeException(); int temp, y=0, b=0x8000, bshft=15, v=x; do { if (v>=(temp=(y<<1)+b<>=1)>0); return y; :omg:====TSI TLFL EEOOLHTG===== ^^^^^^^^^^^^^^^^^ Decode that and you will win.;P ============Hint=========== cout << "33 20 57 4F 52 44 53 62 63 6B 77 6F 72 64 73"; -
void SomeFunction()
{
try
{
//Do all kind of stuff
}
catch(_com_error& e)
{
MessageBox(GetErrorDescription(e));
}
catch(...)
{
MessageBox("Unexpected error. Please contact our tech support");
}
}What modification will you suggest?
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
It depends, but in general it's a bad idea. If you're using MSVC 6 or a later version using the asynchronous exception handling model (see here[^]) it's an extremely bad idea as it will swallow non-C++ exceptions such as access violations and make debugging pointer and memory errors much harder. A general rule of exception handling is that you should only catch what you expect to be thrown. A
catch (…)is a bad construct when rated using this rule as it’s like an admission that you don’t know which exceptions can be thrown. If you do use a catch all you should, in general, re-throw.Steve
-
In our code, if we do a general catch(...), we call a common unhandled exception handler that:
- logs the class name & method name that detected the problem
- displays a message box to the user that an unrecoverable error has occurred
- attempts a graceful shutdown of the application
We never try to continue because we have no idea how the application has been compromised. And should another unhandled exception occur while we are trying to clean up, we just give up and die immediately. [ Hmm... 'Unhandled Exception Handler' -- that just might qualify as an oxymoron! ]
Be clear about the difference between your role as a programmer and as a tester. The tester in you must be suspicious, uncompromising, hostile, and compulsively obsessed with destroying, utterly destroying, the programmer's software. ----- Boris Beizer
If you're using MSVC 6 or a later version using the asynchronous exception handling model (see here[^]) this practice is a bad idea as it means, for one, Dr. Watson can't generate a crash report.
Steve
-
Mr Code Project wrote:
, I would throw the exception again to cause the app to fail or allow some external rountine to handle the exception.
What would be the point in catching the exception there if you are just gonna re-throw it? The exception would find its way up to the calling method regardless and if it has a try/catch block it would be caught by it. There is no point in catching exceptions deep in the core of your code if you are not going to do anything but re-throw it.
=====Brain melting code=====
static int Sqrt(int x){ if (x<0) throw new ArgumentOutOfRangeException(); int temp, y=0, b=0x8000, bshft=15, v=x; do { if (v>=(temp=(y<<1)+b<>=1)>0); return y; :omg:====TSI TLFL EEOOLHTG===== ^^^^^^^^^^^^^^^^^ Decode that and you will win.;P ============Hint=========== cout << "33 20 57 4F 52 44 53 62 63 6B 77 6F 72 64 73";A general rule of exception handling is that you should only catch what you expect to be thrown. Making an application artificially robust does no one any good. For example, say we’ve got a doubly linked list and an exception (not expected) is thrown when we’re setting up the links for a new node. If we have a
catch (…)handler like in the OP then the application will survive with a corrupted linked list structure and instead of an unhandled exception pointing the debugger at the exact problem line as soon as it occurs we’ll have a bizarre error far removed from the actual cause at some later point in time.catch (…)is dangerous if used incorrectly. About the only time it’s OK to use acatch (…)is when you re-throw. The point is that it allows the code to perform some cleanup (so we don’t leak resources) for an exception that isn’t handled locally but is further down the call stack. This is standard (and good) practice.Steve
-
Unless the exceptions are "expected" and have proper "handlers", I would throw the exception again to cause the app to fail or allow some external rountine to handle the exception. Throwing the exception again allows all the callers to free memory, locks, files, etc. as the application gracefully crashes. Without it, you have no clue what your operating state is. - Ian
You get a 5 from me. Having seen first hand the havoc that catch alls can cause I can see the wisdom in this.
Steve
-
It depends, but in general it's a bad idea. If you're using MSVC 6 or a later version using the asynchronous exception handling model (see here[^]) it's an extremely bad idea as it will swallow non-C++ exceptions such as access violations and make debugging pointer and memory errors much harder. A general rule of exception handling is that you should only catch what you expect to be thrown. A
catch (…)is a bad construct when rated using this rule as it’s like an admission that you don’t know which exceptions can be thrown. If you do use a catch all you should, in general, re-throw.Steve
-
In windows, an SEH __try{} __except() will do more good than a catch(...). You get to know more details , about the reason for the exception in this case.
In C++ SEH __try{} __except(), if used incorrectly, can be even more troublesome. For one it doesn't destruct local objects as the stack is being unwound. On top of this all my previous points apply; handling an exception that isn't expected and just ploughing on is a bad idea regardless of the exception handling mechanism used.
Steve
-
A general rule of exception handling is that you should only catch what you expect to be thrown. Making an application artificially robust does no one any good. For example, say we’ve got a doubly linked list and an exception (not expected) is thrown when we’re setting up the links for a new node. If we have a
catch (…)handler like in the OP then the application will survive with a corrupted linked list structure and instead of an unhandled exception pointing the debugger at the exact problem line as soon as it occurs we’ll have a bizarre error far removed from the actual cause at some later point in time.catch (…)is dangerous if used incorrectly. About the only time it’s OK to use acatch (…)is when you re-throw. The point is that it allows the code to perform some cleanup (so we don’t leak resources) for an exception that isn’t handled locally but is further down the call stack. This is standard (and good) practice.Steve
I don't have a lot of experience in C++ but I have plenty in C#. I don't catch exceptions to just re-throw them. If I catch an exception in the lower level parts of my code I will actually do something and then re-throw if necessary. If an exception is thrown and it is caught up at the higher level I can just walk down the call stack to find the source and then add a try/catch block at the higher level code to catch that and do something. /////Low level code/// public DoStuff() { try { ... } catch(Exception ex) { throw ex;//whats the point? The higher level code will catch this regardless if this is here } } public DoStuff() { try { ... } catch(Exception ex) { ...// do stuff throw ex;//thats better } } ///end low level code///
=====Brain melting code=====
static int Sqrt(int x){ if (x<0) throw new ArgumentOutOfRangeException(); int temp, y=0, b=0x8000, bshft=15, v=x; do { if (v>=(temp=(y<<1)+b<>=1)>0); return y; :omg:====TSI TLFL EEOOLHTG===== ^^^^^^^^^^^^^^^^^ Decode that and you will win.;P ============Hint=========== cout << "33 20 57 4F 52 44 53 62 63 6B 77 6F 72 64 73"; -
I don't have a lot of experience in C++ but I have plenty in C#. I don't catch exceptions to just re-throw them. If I catch an exception in the lower level parts of my code I will actually do something and then re-throw if necessary. If an exception is thrown and it is caught up at the higher level I can just walk down the call stack to find the source and then add a try/catch block at the higher level code to catch that and do something. /////Low level code/// public DoStuff() { try { ... } catch(Exception ex) { throw ex;//whats the point? The higher level code will catch this regardless if this is here } } public DoStuff() { try { ... } catch(Exception ex) { ...// do stuff throw ex;//thats better } } ///end low level code///
=====Brain melting code=====
static int Sqrt(int x){ if (x<0) throw new ArgumentOutOfRangeException(); int temp, y=0, b=0x8000, bshft=15, v=x; do { if (v>=(temp=(y<<1)+b<>=1)>0); return y; :omg:====TSI TLFL EEOOLHTG===== ^^^^^^^^^^^^^^^^^ Decode that and you will win.;P ============Hint=========== cout << "33 20 57 4F 52 44 53 62 63 6B 77 6F 72 64 73";The point is to handle local clean up. Here's an example in C++:
HANDLE OpenTheFile(); void CloseTheFile(HANDLE hFile) void SeekToData(HANDLE hFile); int GetData(HANDLE hFile); int GetDataFromFile() { HANDLE hFile = OpenTheFile(); try { SeekToData(hFile); } catch (...) { CloseTheFile(hFile); throw; } int val = GetData(hFile); CloseTheFile(hFile); return val; }In this case we use thecatch (...)to perform local cleanup regardless of the error and re-throw the exception so other error handling, such as displaying an error, can be handled in another function (which called this function, directly of indirectly).Steve
-
The point is to handle local clean up. Here's an example in C++:
HANDLE OpenTheFile(); void CloseTheFile(HANDLE hFile) void SeekToData(HANDLE hFile); int GetData(HANDLE hFile); int GetDataFromFile() { HANDLE hFile = OpenTheFile(); try { SeekToData(hFile); } catch (...) { CloseTheFile(hFile); throw; } int val = GetData(hFile); CloseTheFile(hFile); return val; }In this case we use thecatch (...)to perform local cleanup regardless of the error and re-throw the exception so other error handling, such as displaying an error, can be handled in another function (which called this function, directly of indirectly).Steve
Yes, thats what I was trying to say. My first response was because I thought the guy was saying you should re-throw exceptions even when you don't do anything in the catch block.
=====Brain melting code=====
static int Sqrt(int x){ if (x<0) throw new ArgumentOutOfRangeException(); int temp, y=0, b=0x8000, bshft=15, v=x; do { if (v>=(temp=(y<<1)+b<>=1)>0); return y; :omg:====TSI TLFL EEOOLHTG===== ^^^^^^^^^^^^^^^^^ Decode that and you will win.;P ============Hint=========== cout << "33 20 57 4F 52 44 53 62 63 6B 77 6F 72 64 73"; -
Yes, thats what I was trying to say. My first response was because I thought the guy was saying you should re-throw exceptions even when you don't do anything in the catch block.
=====Brain melting code=====
static int Sqrt(int x){ if (x<0) throw new ArgumentOutOfRangeException(); int temp, y=0, b=0x8000, bshft=15, v=x; do { if (v>=(temp=(y<<1)+b<>=1)>0); return y; :omg:====TSI TLFL EEOOLHTG===== ^^^^^^^^^^^^^^^^^ Decode that and you will win.;P ============Hint=========== cout << "33 20 57 4F 52 44 53 62 63 6B 77 6F 72 64 73";In general, the rules for
catch (...)are like this: - Don't have acatch (...)that does nothing or simply displays an error. This should be done in acatchblock that catches a specific exception. - If you have acatch (...)it should only perfom local cleanup and then re-throw the exception to a higher level. - The last catch block that handles a specific exception should catch a specific type or the exception should be left unhandled and trigger an unhandled exception. Violating these rules results in fragile and hard to maintain code.Steve
-
It depends, but in general it's a bad idea. If you're using MSVC 6 or a later version using the asynchronous exception handling model (see here[^]) it's an extremely bad idea as it will swallow non-C++ exceptions such as access violations and make debugging pointer and memory errors much harder. A general rule of exception handling is that you should only catch what you expect to be thrown. A
catch (…)is a bad construct when rated using this rule as it’s like an admission that you don’t know which exceptions can be thrown. If you do use a catch all you should, in general, re-throw.Steve
Stephen Hewitt wrote:
it's an extremely bad idea as it will swallow non-C++ exceptions such as access violations and make debugging pointer and memory errors much harder.
I second that. It's a PITA for sure. :sigh:
-- Mit viel Oktan und frei von Blei, eine Kraftstoff wie Benziiiiiiin!
-
I don't have a lot of experience in C++ but I have plenty in C#. I don't catch exceptions to just re-throw them. If I catch an exception in the lower level parts of my code I will actually do something and then re-throw if necessary. If an exception is thrown and it is caught up at the higher level I can just walk down the call stack to find the source and then add a try/catch block at the higher level code to catch that and do something. /////Low level code/// public DoStuff() { try { ... } catch(Exception ex) { throw ex;//whats the point? The higher level code will catch this regardless if this is here } } public DoStuff() { try { ... } catch(Exception ex) { ...// do stuff throw ex;//thats better } } ///end low level code///
=====Brain melting code=====
static int Sqrt(int x){ if (x<0) throw new ArgumentOutOfRangeException(); int temp, y=0, b=0x8000, bshft=15, v=x; do { if (v>=(temp=(y<<1)+b<>=1)>0); return y; :omg:====TSI TLFL EEOOLHTG===== ^^^^^^^^^^^^^^^^^ Decode that and you will win.;P ============Hint=========== cout << "33 20 57 4F 52 44 53 62 63 6B 77 6F 72 64 73";Henize wrote:
catch(Exception ex) { ...// do stuff throw ex;//thats better }
This should be: catch(Exception ex) { ...// do stuff throw;//thats better } i.e., throw rather than throw ex otherwise you lose some of the stack trace.
Kevin
-
It depends, but in general it's a bad idea. If you're using MSVC 6 or a later version using the asynchronous exception handling model (see here[^]) it's an extremely bad idea as it will swallow non-C++ exceptions such as access violations and make debugging pointer and memory errors much harder. A general rule of exception handling is that you should only catch what you expect to be thrown. A
catch (…)is a bad construct when rated using this rule as it’s like an admission that you don’t know which exceptions can be thrown. If you do use a catch all you should, in general, re-throw.Steve
Exactly. I was expecting this answer. Many programmer get excited that their program continues to run inspite of an exception like access violation and forget that there might be something dangerous that occured like memory overwrite etc. so the program may not be in a valid state to continue. I had tough time convincing one of the programmer to drop that habit.
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
-
Henize wrote:
catch(Exception ex) { ...// do stuff throw ex;//thats better }
This should be: catch(Exception ex) { ...// do stuff throw;//thats better } i.e., throw rather than throw ex otherwise you lose some of the stack trace.
Kevin
Yeah. I forget about that sometimes:doh:
=====Brain melting code=====
static int Sqrt(int x){ if (x<0) throw new ArgumentOutOfRangeException(); int temp, y=0, b=0x8000, bshft=15, v=x; do { if (v>=(temp=(y<<1)+b<>=1)>0); return y; :omg:====TSI TLFL EEOOLHTG===== ^^^^^^^^^^^^^^^^^ Decode that and you will win.;P ============Hint=========== cout << "33 20 57 4F 52 44 53 62 63 6B 77 6F 72 64 73"; -
Exactly. I was expecting this answer. Many programmer get excited that their program continues to run inspite of an exception like access violation and forget that there might be something dangerous that occured like memory overwrite etc. so the program may not be in a valid state to continue. I had tough time convincing one of the programmer to drop that habit.
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
I've had similar experiences. I've also had to debug code which had bugs caused by such code and in the worst case the problem took weeks to track down.
Steve