views:

132

answers:

3

I have to create a set of wrapping C++ classes around an existing C library.

For many objects of the C library, the construction is done by calling something like britney_spears* create_britney_spears() and the opposite function void free_britney_spears(britney_spears* brit).

If the allocation of a britney_spears fails, create_britney_spears() returns NULL.

This is, as far as I know, a very common pattern.

Now I want to wrap this inside a C++ class.

//britney_spears.hpp

class BritneySpears
{
  public:

    BritneySpears();

  private:

    boost::shared_ptr<britney_spears> m_britney_spears;
};

And here is the implementation:

// britney_spears.cpp

BritneySpears::BritneySpears() :
  m_britney_spears(create_britney_spears(), free_britney_spears)
{
  if (!m_britney_spears)
  {
    // Here I should throw something to abort the construction, but what ??!
  }
}

So the question is in the code sample: What should I throw to abort the constructor ?

I know I can throw almost anything, but I want to know what is usually done. I have no other information about why the allocation failed. Should I create my own exception class ? Is there a std exception for such cases ?

Many thanks.

+5  A: 

You would not want to derive a BritneyFailedToConstruct exception. My experience is that you should keep exception hierarchies as flat as possible (I use one single type per library). The exception should derive from std::exception, and should somehow contain a message that is accessible via std:;exceptions virtual what() function. You then throw it in your constructor:

throw MyError( "failed to create spears object" );

The following is the declaration for the exception class I use in my own utility library:

class Exception : public std::exception {

    public:

        Exception( const std::string & msg = "" );
        Exception( const std::string & msg, int line,
                        const std::string & file );

        ~Exception() throw();

        const char *what() const throw();
        const std::string & Msg() const;

        int Line() const;
        const std::string & File() const;

    private:

        std::string mMsg, mFile;
        int mLine;
};

#define ATHROW( msg )\
{   \
    std::ostringstream os;  \
    os << msg               \
    throw ALib::Exception( os.str(), __LINE__, __FILE__  ); \
}   \

The macro is for conveniently adding the file name and line number, and providing stream formatting for the message. This lets you say things like:

ATHROW( "britney construction failed - bad booty value of " << booty );
anon
You want to be careful where your throw those spears. You'll have someone's eye out!
Johnsyweb
Thanks for your reply. Someone at my office said that I should throw a `std::bad_alloc()` in this very case, but I don't like it. What's your opinion on that ?
ereOn
@ereOn Like I said, I don't like complicated exception schemes. If you catch near the throw site, the type isn't normally important because you know what caused the error. and if you catch far away from the throw site, you can't fix the exact problem anyway. so using lots of different exception types seems self-defeating to me.
anon
It depends on the various causes of failures. If the only way to fail is because there is not enough memory, then `bad_alloc` is the way to go. Otherwise, perhaps could you check `errno`, a global containing the error number in most C programs, and throw an exception depending on the error code ?
Matthieu M.
@Neil: I catch far from the calling site but I prefer to have one exception type (derived from a common base) per error type. Only seems natural (I have a macro to generate the type). This allows me to document my function (what can it throw), and occasionally to catch near the throw site with good discrimination when I want to ignore the error in THIS very particular situation.
Matthieu M.
@Matthieu M: I prefer fewer exceptions (one per library part). The only reason to have multiple exceptions is to allow you to specifically catch a particular exception and this is only useful if there is some specific way to fix the exception. If all you can do is log the exception and abort the operation then it seems a little overkill to provide multiple different types of exception when a generic exception achieves the same result.
Martin York
I understand the rationale, and that is why I use a hierarchy of exception: to allow a higher degree of abstraction (when catching). However I've seen so many times people using a combination of One exception + A collection of error codes (for no other purpose that printing the error code, there's almost never a switch on this code) that I preferred to directly print the name of the error (rather than a magic number) and so as not to lose on the granularity, typed my exception more tightly. It's just that I don't want to miss some granularity that could be necessary.
Matthieu M.
+1  A: 

Throw some abstract exception (like std::exception or derived from std::exception) or use the zombie-state technique as described here.

Note that the second method isn't common but has some pros (as well as cons).

Kotti
A: 

I would either throw a runtime_error (link) or an object of your own class derived from runtime_error.

JohnMcG
r <-> rr :) (15chars)
Billy ONeal
Thanks -- at least I was consistent and had it wrong in both places..
JohnMcG