Tech Off Thread

16 posts

Forum Read Only

This forum has been made read only by the site admins. No new threads or comments can be added.

Error handling in C++

Back to Forum: Tech Off
  • User profile image
    cro

    I don't know how to handle error on a C++ library that we are developping. For exemple, would you prefere no1, or no2 and why ?

    // No 1
    class ListStation
    {
       ...
       void Add(Station* station)
       {
          // Preconditions
          for (unsigned int index = 0; index < _stations.size(); ++index)
          {
             if (station._name == _stations[index]._name)
             {
                throw std::invalid_argument();
             }
          }
          ...
       }
    };

    // No 2
    class ListStation
    {
       ...
       bool Add(Station* station)
       {
          // Preconditions
          for (unsigned int index = 0; index < _stations.size(); ++index)
          {
             if (station._name == _stations[index]._name)
             {
                return false;
             }
          }
          ...
          return true;
       }
    };

  • User profile image
    mig

    To start things off, you should be using iterators, and you should dereference pointers, but I'm guessing you just typed this up real quick to ask us to choose between exceptions and return values...

    I would go with the exception route, because if something is exceptional it forces the caller of your library to handle it, where if you return false the caller can simply ignore that. Granted that the caller can also swallow your exception, so I guess it really depends on wether you need to take advantage of some of the goodies that C++ Exceptions give you, like stack unwinding of objects and an object oriented hierarchy of error classes (which naturally you can and should extend).

    You might also want to do a Google search for articles and tutorials, I found one but didn't read it, you can find it here.

    My .02cents.

  • User profile image
    RichardRudek

    Depends upon how important the error is. Generally, one would go with the return value up and until you detemine that the failure of the particular subroutine is critically important.

    My main reason for saying this is that you, generally, can't "continue" after an exception. As in:

    try
    {
        ... // 1
        ... // 2 exception thrown here, mid-block
        ... // 3

    }
    catch ... {} // no continuation possible (from 3 onwards): Well I haven't been able to...

    which means that if the subroutine in not a critical failure, you have to reduce the size of the code block, just for the "arrogant" subroutine:

    try { arrogant_sub(); }
    catch {}

    [A]

  • User profile image
    Massif

    Like others have said, it depends on the situation to a certain extent.

    But if the errors you're talking about are resulting from incorrect use of your library then I'd fail hard and fast with the exception. Exceptions are good for this sort of thing, provided they truly are exceptional - if you're anticipating certain kinds of error being common then use a return, but make sure it's one which is very clearly an error code.

    I (and this is my opinion) don't like to return boolean to indicate success or failure, I tend to opt for some sort of success enum - which at least gives me the option of describing what went wrong.

    In short:
    If error caused by idiot developer -> throw exception
    else -> return failure

    there's no rule against having both failure modes in the same code, if they're in the right context.

  • User profile image
    Randolpho

    Definitely go the exception route. It's a much better means handling errors, IMO.

    RichardRudek wrote:

    try { arrogant_sub(); }
    catch {}


    NEVER NEVER NEVER DO THIS!!!!!!

    This a WTF of the most extreme type, creating a bug that is very difficult to track down!

    Exceptions happen for a reason. You should either a) handle them and deal with their consequences, or b) let them filter up the execution stack and make the caller (or its caller, etc.) handle them.

  • User profile image
    RichardRudek

    Randolpho wrote:
    Definitely go the exception route. It's a much better means handling errors, IMO.

    RichardRudek wrote:
    try { arrogant_sub(); }
    catch {}


    NEVER NEVER NEVER DO THIS!!!!!!

    This a WTF of the most extreme type, creating a bug that is very difficult to track down!

    Exceptions happen for a reason. You should either a) handle them and deal with their consequences, or b) let them filter up the execution stack and make the caller (or its caller, etc.) handle them.


    You had me scratching my head, for a bit. Then I realised that you were taking the catch {} literally.

    Yes, DO NOT just consume/absorb exceptions [1]. That is not what I meant - I simply got lazy when typing that out. Thanks for pointing that out... Smiley

    [1] If the exception is really of no importance, then if you decide to absorb it, make sure you at least comment it so that it stands out. eg Using comments like:  // HACK: , etc. 
    Quick quiz: who uses the CLR's Int32.TryParse(), and who uses Int32.Parse() ?  Is your descision based on the surrounding code, or is it a religious stance ?

    PS: Yes, I am aware of the NT Kernels use of exceptions for various Paging mechanisms. Which is why catch(...) is bad !

  • User profile image
    Randolpho

    RichardRudek wrote:
    You had me scratching my head, for a bit. Then I realised that you were taking the catch {} literally.

    Yes, DO NOT just consume/absorb exceptions [1]. That is not what I meant - I simply got lazy when typing that out. Thanks for pointing that out...


    Ahhh..... Whew. I'm sorry, I try to stamp out bad code wherever I go, and absorbing an exception is always bad.

    RichardRudek wrote:
    PS: Yes, I am aware of the NT Kernels use of exceptions for various Paging mechanisms. Which is why catch(...) is bad !


    catch(...) isn't always bad, provided you rethrow the original exception. In fact, sometimes it's 100% necessary to release resources.

  • User profile image
    Sven Groot

    Randolpho wrote:
    
    RichardRudek wrote: You had me scratching my head, for a bit. Then I realised that you were taking the catch {} literally.

    Yes, DO NOT just consume/absorb exceptions [1]. That is not what I meant - I simply got lazy when typing that out. Thanks for pointing that out...


    Ahhh..... Whew. I'm sorry, I try to stamp out bad code wherever I go, and absorbing an exception is always bad.

    PS: Yes, I am aware of the NT Kernels use of exceptions for various Paging mechanisms. Which is why catch(...) is bad !


    catch(...) isn't always bad, provided you rethrow the original exception. In fact, sometimes it's 100% necessary to release resources.

    With the right combination of compiler flags catch(...) catches SEH exceptions (although I believe that's no longer the default), and there's usually not anything sensible you can do at that point, usually not even clean up resources.

    I don't think there's anything wrong with swallowing an exception, provided you're swalling a specific exception (so not a catch(...)). If you have a certain kind of exception that you know you don't care about, and there's no better way to discover the conditions under which it occurs beforehand, I see nothing wrong with it.

  • User profile image
    cro

    We will go with the exception, thanks all. Let's say that you have a parameter that range from 0 to 100. You would do something like this :

    void SetHumiditeRelative(const float humiditeRelative)
    {
       // PrĂ©conditions
       if ( ! (humiditeRelative  >= 0.0f && humiditeRelative <= 100.0f))
       {
          throw out_of_range("");
       }
       ...
    }

    But what parameter do you give to the exception ? The fonction name, the out of range parameter, the correct range for the parameter ?

  • User profile image
    Randolpho

    cro wrote:
    We will go with the exception, thanks all. Let's say that you have a parameter that range from 0 to 100. You would do something like this :

    void SetHumiditeRelative(const float humiditeRelative)
    {
       // PrĂ©conditions
       if ( ! (humiditeRelative  >= 0.0f && humiditeRelative <= 100.0f))
       {
          throw out_of_range("");
       }
       ...
    }

    But what parameter do you give to the exception ? The fonction name, the out of range parameter, the correct range for the parameter ?



    out_of_range is exactly what you should throw.

    the parameter to any exception is supposed to be a human-readable message that explains why the error occured. Sometimes you want to show the value inside the message, but sometimes (usually for purposes of security) you do not. It's really up to you.

    If you try to embed the parameter value into the message, you'll have to build the message somehow; probably using a stringstream. Personally, I'd skip it and pass an ungenerated string literal, something along the lines of "HumiditeRelative must have a value between 0.0 and 100.0"

  • User profile image
    cro

    Our library is C++ standard open source under lgpl. But we also have a GUI software that use our library. The software is in french but we will have an english version. So for the human readable exception, how do you handle multiple language in the GUI ?

  • User profile image
    AndyC

    cro wrote:
    

    Our library is C++ standard open source under lgpl. But we also have a GUI software that use our library. The software is in french but we will have an english version. So for the human readable exception, how do you handle multiple language in the GUI ?



    Put a localized string in at runtime. You should never rely on the contents of the exception objects parameter for anything other than display data. If you need to be able to distinguish them, create separate exception classes.

  • User profile image
    Randolpho

    cro wrote:
    

    Our library is C++ standard open source under lgpl. But we also have a GUI software that use our library. The software is in french but we will have an english version. So for the human readable exception, how do you handle multiple language in the GUI ?



    Very rarely should the value of an exception be displayed directly to the user. By human readable, I meant that it should be something the developer can read while debugging the program.

    Because you're building a library that might be used by others, AndyC's suggestion of a localized string is perfectly valid. C++/STL doesn't have a standardized means of localization, so you'll have to roll your own string table, but it's quite doable.

    That said, I think it's probably better to go with some single standard language and skip internationalization. If you'll mostly be used by French developers, stick to French. If you'll mostly be used by foreign markets, however, opt for strictly English.

  • User profile image
    cro

    So the UI should never display exception error message, and if this happens (it will), at least it would be in french and we (developpers) will know why that exception was throw ? I must said that I like the idea !

  • User profile image
    Randolpho


    cro wrote:
    

    So the UI should never display exception error message, and if this happens (it will), at least it would be in french and we (developpers) will know why that exception was throw ? I must said that I like the idea !



    The exception should be human readable by the developer -- the person *using* your library, calling your method, etc. If this is an internal library, then, yeah, French is fine. If you're planning to distribute the library, you need to keep your target audience in mind. If they're mostly French speaking developers, then French is still fine. If they're not, you'd probably be better off using English, since that's the defacto international/common language.

  • User profile image
    ST

    Whether to use return value (true/false) or to throw and exception depends on the case.

    Generally, exceptions are thrown if something exceptionally happened. For example, the communication link was dropped and you cannot send/receive more data.

    In your example code, you are trying to add an object to an array. If an object with this name already exest, should you throw exception or return false? Personally, I would return false. There is nothing exceptional that someone is trying to add an object twice to your array. If the caller is aware of this it can check the returned value.


    Best regards,
    ST

Conversation locked

This conversation has been locked by the site admins. No new comments can be made.