Proper way of handling trivial exceptions?

[gabars]

Hi, I've been programming as a hobby for a couple of years. I learned from online courses and documentation and I just came across something I don't even know how to explain, I think maybe that's because I'm doing it wrong...

I'm writing a library to handle a deck of cards, with a class "deck" and a class "card". I have a function which transfers some cards* from a vector (pile) to another one (hand), and when the pile doesn't contain enough cards, I throw an exception, so I can catch it and decide what I do then (depending on the game, it could end or I could shuffle the cards again).

void pass_cards(unsigned int c,std::vector<card*> *to,std::vector<card*> *from) throw(int)
{
  if(from->size()<c)
  {
    throw 1;
  }
  for(unsigned int i=0;i!=c;i++)
  {
    to->push_back(from->back());
    from->pop_back();
  }
  return;
}

And this is how I use it:

    cout<<"Hand:"<<endl;
    try
    {
      hand1.clear();
      pass_cards(5,&hand1,&pile);
    }
    catch(int e)
    {
      err=e;
      if(err==1)
        cout<<"Pile empty"<<endl;

    }

It would be simpler return an int instead of throwing it (don't know why I didn't) but I feel like that's not the right way either. How would you do this?

[ultifinitus]

You have a ton of options:

1. return an int value, 1 on success, -1 on failure, -x on specific failure. Typical libraries have a function that you can pass the last error into to get a message back for the user or for yourself. You can make a global enum that has error codes.

2. return a struct that includes a value and a description if there is an error.

3. use a "state" object which you pass into the function that includes information about successes or failures.

4. use an event system (nice for multithreaded programs) that sends a specific message back to the main program with the negative response from the function.

Heck there are probably more options involving exceptions like you're using. My motto in programming is don't judge folks for how they program if it works, even if it pains me when taking over legacy code..

Everyone has a different style, just stay consistent and you'll be fine :)

[gabars]

Thank you, I really like #2. I'll definitely use it later. Still open to other ideas though :)

[Disch]

The only thing you should ever throw is a class derived from std::exception. I know the language allows you to throw other things, but you generally shouldn't.

Deriving classes is good because:

1) It allows you to do a catch() based on a specific type of exception.
2) std::exception is bundled with a what() function which provides a textual explanation of what went wrong.


So the exception basically is your struct that has the error code with text.


EDIT:

Example:

#include <stdexcept>

// ...

void pass_cards(unsigned int c,std::vector<card*> *to,std::vector<card*> *from) throw(int)
{
  if(from->size()<c)
  {
    throw std::underflow_error("Unable to pass cards because the pile was empty.");
  }
  for(unsigned int i=0;i!=c;i++)
  {
    to->push_back(from->back());
    from->pop_back();
  }
  return;
}

// ...

    cout<<"Hand:"<<endl;
    try
    {
      hand1.clear();
      pass_cards(5,&hand1,&pile);
    }
    catch(std::exception& e)
    {
        cout << e.what() << endl;
    }

[gabars]

Nice, that's almost what I ended up doing. I made a class derived from std::exception and implemented the what() function. I didn't include <stdexcept> though and it works... Should I?

The only thing you should ever throw is a class derived from std::exception. I know the language allows you to throw other things, but you generally shouldn't.

Thanks for the tip :)

I put it on pastebin if you want to take a look at it
http://pastebin.com/ZcLR0Nun

[Disch]

I made a class derived from std::exception and implemented the what() function.

You shouldn't have to re-implement the what() function. You can just use the parent class's version:

class MyException : public std::exception
{
public:
    MyException(const std::string& msg) : exception(msg) {}

    // no need to re-implement what() here... it will work fine without it.  As long as you
    //   forward the string 'msg' to the std::exception ctor as I do above.
};

I didn't include <stdexcept> though and it works... Should I?

<stdexcept> defines several common exception types like runtime_error, underflow_error, etc. If you're not using those, you don't need to include that header. I just used it in my example because I was using underflow_error.

[JLBorges]

> I didn't include <stdexcept> though and it works... Should I?

To use std::exception, you need to #include <exception>
(Or include one of the headers that includes <exception>)

> class MyException : public std::exception
> {
> public:
>    MyException(const std::string& msg) : std::exception(msg) {}

This won't compile. std::exception does not have a constructor that accepts a std::string
Ideally an exception class derived directly from std::exception should override what() - the (default constructed) base class object's what() would return an empty string.
http://en.cppreference.com/w/cpp/error/exception/exception

Favour inheriting the program's exception classes from either std::logic_error or std::runtime_error or from a class derived from one of them.

Favour constructing these exception objects using a constructor accepting a const char* over one accepting a const std::string& (C++11)
See: http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-defects.html#254

Favour inheriting from exception base classes virtually.
See: http://www.boost.org/doc/libs/1_57_0/libs/exception/doc/using_virtual_inheritance_in_exception_types.html

[Disch]

This won't compile. std::exception does not have a constructor that accepts a std::string

Whoops! I assumed it did. I usually derive from runtime_error which does has such a ctor.

[gabars]

Thank you both, I switched to inheriting virtually from runtime_error now.

Favour inheriting from exception base classes virtually.

Is that so I can catch(std::exception) and what() will still work?

[LB]

Virtual inheritance means that when a class derives from your class, your class is no longer responsible for calling the constructor of the class you virtually inherited from; the deriving class is. This way you don't need to duplicate constructors all the way through the class hierarchy.

As an added benefit, it enables proper multiple inheritance (which is pretty controversial, by the way).

In the case of exceptions, this means you don't need to channel the message through each level of inheritance - the most derived class can call the base exception's constructor directly, and the rest of the ctor calls are to default ctors.

[JLBorges]

> Favour inheriting from exception base classes virtually
>> Is that so I can catch(std::exception) and what() will still work?

Yes, yes.

We want exception derived classes to be catchable by handlers for exception base classes.

When multiple inheritance is involved, inheriting virtually avoids the ambiguity in conversion from a derived class to a base class. (This ambiguity would result in an exception of a derived class type not being caught by the handler for the exception of the base class type.)

This small program illustrates the difference between the two:

#include <stdexcept>
#include <iostream>

namespace bad // non-virtual inheritance from exception base class
{
    struct file_open_error : std::runtime_error
    { file_open_error() : std::runtime_error( "failed to open file" ) {} };

    struct network_error : std::runtime_error
    { network_error() : std::runtime_error( "unable to access network" ) {} };

    void foo()
    {
        // try to open file on a network server; on failure
        {
            struct network_file_error final : file_open_error, network_error
            { network_file_error() {} };

            throw network_file_error() ;
        }
    }
}

namespace good // virtual inheritance from exception base class
{
    struct file_open_error : virtual std::runtime_error
    { file_open_error() : std::runtime_error( "failed to open file" ) {} };

    struct network_error: virtual std::runtime_error
    { network_error() : std::runtime_error( "unable to access network" ) {} };

    void foo()
    {
        // try to open file on a network server; on failure
        {
            struct network_file_error final : file_open_error, network_error
            { network_file_error() : std::runtime_error( "unable to access network => failed to open file" ) {} };

            throw network_file_error() ;
        }
    }
}

int main()
{
    try { std::cout << "this (non-virtual inheritance from exception base class) is bad:\n  " ; bad::foo() ; }

    catch( const std::exception& e ) // won't be caught here: ambiguous conversion to std::exception
    { std::cout << "caught std::exception what: '" << e.what() << "'\n" ; }

    catch(...) // but caught here
    { std::cout << "caught some unknown error\n" ; }

    std::cout << "------------------------------\n" ;

    try { std::cout << "this (virtual inheritance from exception base class) is what we want:\n  " ; good::foo() ; }

    catch( const std::exception& e ) // will be caught here: unambiguous conversion to std::exception
    { std::cout << "caught std::exception what: '" << e.what() << "'\n" ; }

    catch(...)
    { std::cout << "caught some unknown error\n" ; }
}

clang++ -std=c++14 -stdlib=libc++ -O3 -Wall -Wextra -pedantic-errors main.cpp -lsupc++ && ./a.out 

this (non-virtual inheritance from exception base class) is bad:
  caught some unknown error
------------------------------
this (virtual inheritance from exception base class) is what we want:
  caught std::exception what: 'unable to access network => failed to open file'

http://coliru.stacked-crooked.com/a/d0de00bbf4199896

[gabars]

Wow thanks! I'll keep that in mind.