debugcppMinor
C++ custom exception design
Viewed 0 times
customdesignexception
Problem
I have a custom exception hierarchy in C++ to detect unexpected cases in a library I am implementing. The base exception class inherits from
The header file for the base exception class is as follows:
And here is the header file for one of the derived classes, which is supposed to catch any process related error (I am spawning multiple processes within C++ in the source code, and I am using this class there for error handling):
So one would invoke an error case for the derived class above by writing
std::exception and all other exception classes derive from this base class.The header file for the base exception class is as follows:
#ifndef LibException_H_
#define LibException_H_
#include
class LibException : public std::exception
{
public:
// Sets member_err to err
LibException(const char* err);
virtual ~LibException() throw ();
// Returns member_err.
virtual const char* what() const throw();
protected:
// A description of the error. Should probably set this to private.
const char* member_err;
private:
};
#endifAnd here is the header file for one of the derived classes, which is supposed to catch any process related error (I am spawning multiple processes within C++ in the source code, and I am using this class there for error handling):
#ifndef ProcessException_H_
#define ProcessException_H_
#include "Includes/LibException.H"
class ProcessException : public LibException
{
public:
enum Reason
{
NETWORK,
IO,
OTHER,
};
// Calls superclass(err) and sets the two member variables
ProcessException(const char* err, Reason reason,
const char* process_command);
virtual ~ProcessException() throw ();
// Returns a char* pointer to the reason.
//
// E.g. NETWORK = "NETWORK"
// @return: member_reason.
const char* getReasonChar() const throw();
// Returns a char* pointer to the spawned process' command (e.g. "ls -a").
//
// @return: member_command.
const char* getCommand() const throw();
protected:
private:
Reason member_reason;
const char* member_command;
};So one would invoke an error case for the derived class above by writing
throw ProcessException("Process exited unexpectedly", ProcessException::NETWORK, process_command); where process_command is the string representation for a certain process (e.g. ls -a).Solution
You should probably inherit from
The
There is no need for a virtual destructor. That is already taken care of in the base class
This seems obtuse:
Why not just return
I personally don't like interfaces that take pointers. There is no ownership semantics associated with the pointer and thus people can mistakenly provide the wrong type of pointer (ie a dynamically allocated one in this case). So I would change the interface to be
Edit: Moreover I have another exception class called ParseException (very similar to the derived class above) which is thrown when a parser that is parsing a file or a command from the shell fails unexpectedly.
I think the key phrase here is
I have read that such errors are better handled via assert statements because they don't represent "exceptional" cases.
Asserts are the worst type of control and they are really only good for debugging; not for error handling. Remember
Could anyone please comment on this?
Control flow is usually not best handled with exceptions. But everything is situational. If you find it useful feature to use then go ahead.
Creating your Own exceptions
I used to create exceptions for different situations but found I never used them. So I have started simply using
The trouble is the definition of "usefully caught". This not only means that the exception needs to carry information that I can use to fix the problem but the library itself must provide a public API that will allow me to fix the situation.
If all you can do with the exception is log it (including dump to std::cout). Then is it really a useful exception to catch?
std::runtime_error rather than std::exception.The
std::exception and all its descendants already implement what() so there is no need to implement that in your code. So you can remove these:// Returns member_err.
virtual const char* what() const throw();
protected:
// A description of the error. Should probably set this to private.
const char* member_err;There is no need for a virtual destructor. That is already taken care of in the base class
std::exception. Since you are not doing memory management (your example shows string literals) these methods are empty anyway. So you can remove this:virtual ~LibException() throw ();
virtual ~ProcessException() throw ();This seems obtuse:
// Returns a char* pointer to the reason.
//
// E.g. NETWORK = "NETWORK"
// @return: member_reason.
const char* getReasonChar() const throw();Why not just return
Reason?I personally don't like interfaces that take pointers. There is no ownership semantics associated with the pointer and thus people can mistakenly provide the wrong type of pointer (ie a dynamically allocated one in this case). So I would change the interface to be
std::string const& this guarantees the correct semantics are used (and you can still use the string literal). This also jibes with the std::exception (and family) which all accept a std::string for the constructor.LibException(std::string const& err);Edit: Moreover I have another exception class called ParseException (very similar to the derived class above) which is thrown when a parser that is parsing a file or a command from the shell fails unexpectedly.
I think the key phrase here is
fails unexpectedly. If its unexpected to fail for this reason then its an exception.I have read that such errors are better handled via assert statements because they don't represent "exceptional" cases.
Asserts are the worst type of control and they are really only good for debugging; not for error handling. Remember
assert() compiles to the NO-OP when in release mode so there is no checks.Could anyone please comment on this?
Control flow is usually not best handled with exceptions. But everything is situational. If you find it useful feature to use then go ahead.
Creating your Own exceptions
I used to create exceptions for different situations but found I never used them. So I have started simply using
std::runtime_error. The only time I define a specific exception is when it can be usefully caught outside the library layer I am throwing from.The trouble is the definition of "usefully caught". This not only means that the exception needs to carry information that I can use to fix the problem but the library itself must provide a public API that will allow me to fix the situation.
If all you can do with the exception is log it (including dump to std::cout). Then is it really a useful exception to catch?
Code Snippets
// Returns member_err.
virtual const char* what() const throw();
protected:
// A description of the error. Should probably set this to private.
const char* member_err;virtual ~LibException() throw ();
virtual ~ProcessException() throw ();// Returns a char* pointer to the reason.
//
// E.g. NETWORK = "NETWORK"
// @return: <const char*> member_reason.
const char* getReasonChar() const throw();LibException(std::string const& err);Context
StackExchange Code Review Q#160671, answer score: 9
Revisions (0)
No revisions yet.