Advertisement

Error handling: Opinions?

Started by September 27, 2000 02:29 PM
43 comments, last by Aienthiwan 24 years, 3 months ago
quote:

you don''t have to use auto pointers to use exceptions



True, you don''t NEED auto pointers to use exceptions but not using them when using exceptions will either cause memory leaks or duplicated/poorly written code. Also, there are other "issues" with exceptions that you must code for unless you want memory leaks, such as specifically coding both your constructors and deconstructors for cases where exceptions are thrown during execution of either, or memory leaks will occur.

Scott Meyer''s More Effective C++ gives an example of all these situations I''m talking about.

quote:

Exceptions give you more control over your code, and let you separate error handling code from code to perform actions when there are no errors.



True, but exceptions also make your code much harder to debug, IMO. It makes it absolutely impossible to look at a section of code on hardcopy (paper) and know what''s going on. You have no clue if this function that is being called can throw an exception, where the exception will be caught, etc. You basically need all of the code to follow a programs normal operation. With return codes it is in plain english (well, maybe not ) and you know exactly what happens when an error is encountered and what line of code will be executed next. Exceptions remind me of goto''s on steroids (ok, maybe not, but I still had to throw in how exceptions perform the one function that people damn goto''s for ).

quote:

if you catch a truely evil exception (I''m really not even sure anything that bad would leave your computer functioning), and give a useful message, fine, but for most errors just making the program exit is utterly unacceptable.



But I thought exceptions are only supposed to be used for "exceptional" cases, where the program hits an error where the program can''t continue (say, you are unable to initialize DDraw). There would be no reason not to display an error message and exit the program.

As far as simple errors like you were unable to save the game (perhaps because of diskspace) then I personally see no need whatsoever to USE exceptions in this case. Simply print out an error message and exit the function and let your program continue as normal.

Of course, I hardly use exceptions, I admit it. One of the reasons is because of all the problems and things you must code around when using exceptions to avoid memory leaks. If I''m missing something or am mis-informed then don''t hesitate to say something. These type of "arguments" are the best way to learn things, IMO


- Houdini
- Houdini
I love circular discussions. =)

First thing, I think the "source" tags are broken, because
now things seem to be leading off the edge of my browser since
the post that went past the edge of the screen within source
tags.

OK, for those who are nay-saying exceptions, I ask you to give
them a shot. I just spent a majority of my work week converting
a program to use exceptions instead of the prior way, and things
are much simpler.

(note, this will be a little long, so be prepared)
I'm an engineer at a communications company, and am responsible
for this piece of legacy software. This software is fairly
complicated (~600k in source code, not counting resources or
libraries). It allows users to test hardware that we also build
(yes, I'm an engineer, so I do both) by writing script
programs. So the program is 1/2 script parser/interpretter, 1/2
hardware control. The script is not compiled, it's interpretted
in real-time.

Let me give you an example function called "bif_allocate_device",
which tries to allocate a piece of hardware. Error conditions
that could occur are:
- the wrong usage was called (improper number of arguments)
- the handle the user wants to use to allocate is already taken
- an invalid hardware descriptor is given
- the hardware is unavailable

Note that the first three errors would be because of the user,
the next is because of some external cause. (BTW bif is
shorthand for "built-in-function")

First, I want to look at the code that calls the function, then
I'll look at the function itself. This is the old way,
when "exceptions were evil":
// first, the code snippet that calls the function// this occurs in a parser, which finds the center term of some// token array (i.e. the thing that needs to be done).// when we get to this point, tokens = "allocate_device"<br>  Bif* pBif = find_bif (tokens); // finds function pointer<br>  if (pBif == NULL)<br>  {<br>    app->logger->write ("ERROR: parser: couldn't find function "<br>      "%s", tokens, LOG_LEVEL_CRITICAL);<br>    return -1;<br>  }<br>  if (script->*pBif (tokens, num_args) == -1) // call function<br>    return -1; // abort script if error, don't print anything<br>               // because script function already did<br><br>// now, here's the function it calls<br>int bif_allocate_device (const TokenArray& tokens, int num_args)<br>{<br>  // check usage<br>  if (num_args < 2)<br>  {<br>    app->logger->write ("ERROR: allocate_device: usage: "<br>      "…print proper usage here…", LOG_LEVEL_CRITICAL);<br>    return -1;<br>  }<br><br>  // evaluate parameters<br>  Param params; // class that encapsulates parameters<br>  int result = evaluate_all_parameters (tokens, num_args, params);<br>  if (result != -1)<br>  {<br>    app->logger->write ("ERROR: allocate_device: problem "<br>      "with parameter #%d", result, LOG_LEVEL_CRITICAL);<br>    return -1;<br>  }<br><br>  // find where in map we'll insert this handle<br>  int handle = atoi (params[0]); // get handle<br>  DeviceMap::iterator it = devMap.lower_bound (handle);<br><br>  // check that handle doesn't already exist<br>  if (it != devMap.end () && it->first == handle)<br>  {<br>    app->logger->write ("ERROR: allocate_device: handle %d "<br>      "already in use", handle, LOG_LEVEL_CRITICAL);<br>    return -1;<br>  }<br><br>  // allocate the hardware given the hardware string (2nd param)<br>  Device* pDev = new Device (params[1]);<br>  if (!pDev->isOk ())<br>  {<br>    app->logger->write ("ERROR: allocate_device: couldn't "<br>      "allocate device %s", params[1], LOG_LEVEL_CRITICAL);<br>    delete pDev; // clean-up object<br>    return -1;<br>  }<br><br>  // insert device into map<br>  devMap.insert (it, pDev);<br>  return 0; // success!<br>}<br>    </pre>    <br><br>Quite a mess, isn't it?  The logger is a class that outputs<br>messages to the log window (it's an MFC app, so it's basically a<br>text box).<br><br>Now, let's look at the same function when I decided to use<br>exceptions:<br><pre><br>// first, the caller<br>  try<br>  {<br>    Bif* pBif = find_bif (tokens)<br>    script->*pBif (tokens, num_args);<br>  }<br>  catch (exception& x)<br>  {<br>    CString err;<br>    err.Format ("ERROR: %s: %s", tokens, x.what ());<br>    app->logger->write (err, LOG_LEVEL_CRITICAL);<br>    return -1;<br>  }<br><br>// now the function<br>void bif_allocate_device (const TokenArray& tokens, int num_args)<br>{<br>  // check usage<br>  if (num_args < 2)<br>    throw script_error ("…usage…");<br><br>  // evaluate parameters<br>  Param params; // class that encapsulates parameters<br>  evaluate_all_parameters (tokens, num_args, params);<br><br>  // find where in map we'll insert this handle<br>  int handle = atoi (params[0]); // get handle<br>  DeviceMap::iterator it = devMap.lower_bound (handle);<br><br>  // check that handle doesn't already exist<br>  if (it != devMap.end () && it->first == handle)<br>    throw script_error ("handle %d already in use", handle); <br><br>  // allocate the hardware given the hardware string (2nd param)<br>  Device* pDev = new Device (params[1]);<br><br>  // insert device into map<br>  devMap.insert (it, pDev);<br>}<br>    </pre>    <br><br>How much easier to read is that?  Half of you there think this<br>is cool, while the other half are getting the creeps because<br>"things are going on that aren't explicit".  Damn straight.<br><br>Looking at the code, you probably don't realize ('cause I didn't<br>show it) that these things can also throw exceptions:<br>- find_bif<br>- evaluate_all_parameters<br>- Device::Device<br><br>I understand your unease that you can't see these functions<br>explicitly stopping execution.  But if you look at the bottom<br>example verses the top, you'll see that the true functionality<br>of the code is much more apparent, and not addled with these<br>error-checking statements all over the place.  The top version<br>has about a 2:1 ratio of actual code to error-checking code.<br><br>Some benefits of this specific example:<br>- each error is formatted the exact same way programatically.<br>Without this exception mechanism, it's up to each piece of code<br>to keep with a formatting standard.  Now, each piece just throws<br>its reason for error, and the catch block formats it.<br>- I had to use my bif's return value for error-checking before.<br>Now I can use it for something else if I want to.  I don't have<br>to waste a parameter or return value on error-checking.<br>- Just look at how much cleaner the bottom example is from the<br>top.<br><br>It's important to note that a nifty side-effect of using<br>exceptions is that it forces you to start allocating everything<br>on the stack instead of the heap.  The incentive is that you<br>don't want to have to do any cleanup at all before throwing.<br>Sometimes this isn't possible (so you use auto_ptr), but for<br>the most case it enforces a style of "use stack objects, not<br>pointers."  I think most here will agree that this is the way<br>to go anyway, and provides you with more robust code.  So not<br>only does using exceptions clean up your code; it encourages you<br>to adopt a better object creation style!<br><br>This is a real-life example of how exceptions help your code.  If<br>this doesn't convince you to at least try them out, I don't know<br>what will.<br><br>I SWEAR this is all I have to say on the subject.  Well, mostly.<br>=)<br><br>Edited by - Stoffel on October 5, 2000 12:02:33 PM    
Advertisement
quote: Original post by Houdini
Also, there are other "issues" with exceptions that you must code for unless you want memory leaks, such as specifically coding both your constructors and deconstructors for cases where exceptions are thrown during execution of either, or memory leaks will occur.


Well, first of all, throwing an exception in a destructor is very bad voodoo. It leaves the state of execution in an extremely undefined state, making further execution very undefined.

Secondly, exceptions force you to make resource responsibility explicit. If you adopt the "construction is initialization/allocation" idiom, then all your resources will be freed when an exception is thrown.

Writing exception safe code isn''t hard, it''s just a different mode of thinking. You don''t need to code around anything; just restructure your code so that every resource has an owner (or multiple, in the ref-counted case).

MSN
Yes, your exception way makes it much cleaner, but I don't think
you did the first way the best either. This is basically how I
would have written it:

// first the caller  Bif* pBif = find_bif (tokens); // finds function pointer<br>  if (!pBif)<br>     return FALSE;<br><br>  if (!script->*pBif (tokens, num_args)) // call function<br>    return FALSE; // abort script if error, don't print anything<br>               // because script function already did<br><br>// now the function<br>BOOL bif_allocate_device (const TokenArray& tokens, int num_args)<br>{<br>  // check usage<br>  if (num_args < 2)<br>    RETURN_ERROR("…usage…");<br>  <br>  // evaluate parameters<br>  Param params; // class that encapsulates parameters<br>  <br>  if (!evaluate_all_parameters (tokens, num_args, params))<br>    return FALSE;<br><br>  // find where in map we'll insert this handle<br>  int handle = atoi (params[0]); // get handle<br><br>  DeviceMap::iterator it = devMap.lower_bound (handle);<br><br>  // check that handle doesn't already exist<br>  if (it != devMap.end () && it->first == handle)<br>    RETURN_ERROR("handle %d already in use", handle);<br><br>  // allocate the hardware given the hardware string (2nd param)<br>  Device* pDev = new Device (params[1]);<br><br>  if (!pDev->isOk ()) {<br>     delete pDev;<br>     return FALSE;<br>  }<br><br>  // insert device into map<br>  devMap.insert (it, pDev);<br><br>  return TRUE; // success!<br>}<br><br>  </pre>      <br><br>Notice that all errors that happen inside called functions I <br>actually log inside those functions.  They only return FALSE <br>because the function failed.  This way error logging isn't <br>duplicated everytime a function is called, plus it helps for <br>code reuse (I usually use all classes, and include error logging <br>and any class specific error messages inside the class).  I <br>noticed you are throwing the exceptions (with error messages I'm <br>sure) in those functions, so why wouldn't you do the same if <br>you didn't use exceptions?<br><br>Also, I use a few macros to make the code cleaner plus they <br>are needed to track file/line numbers on errors.<br><br>I think my version is what, 2 or 3 lines longer than your <br>exception version?  And now people who've never seen my code <br>KNOW that I'm checking for errors, and they KNOW when I'm <br>quitting the function because an error occured.  Very explicit <br>and easy to read, IMO.<br><br>And don't forget you PAY (in code size) for every try AND every <br>exception specification you have in your program.  According to <br>Scott Meyer you can expect your overall code size to increase by <br>5-10% if you use try blocks.  And exception specifications <br>generally incur about the SAME penalty as a try block.  If you <br>have a very large program, this can be a huge increase in size.<br><br>I still stick to my statement that returns should be used for <br>normal errors and exception handling should be used <br>for "exceptional" errors, ie, this error can/will cause the <br>program to crash.<br><br>- Houdini<br><br><br>Edited by - Houdini on October 5, 2000 1:24:18 PM    
- Houdini
quote: Original post by Houdini

I still stick to my statement that returns should be used for
normal errors and exception handling should be used
for "exceptional" errors, ie, this error can/will cause the
program to crash.



I rather like the fact that I can write a small console app that tests some ideas I''ve come up with and not having to check the damn return values from functions just to make sure it runs correctly. If it''s going to fail, throw an exception. If not, don''t return false to indicate errors.

I''m sick of allocators returning null on failure. If I knew it would fail I''d either wrap it in a try/catch block or just not care.

MSN

This topic is closed to new replies.

Advertisement