debugcppMinor
Self-implemented C++ exception class
Viewed 0 times
implementedclassexceptionself
Problem
I wrote my own exception class, deriving from
The only thing I see is the deep-copy in the copy-constructor, which is not efficient, when there are many nested exceptions. But exceptions should be rare and not be nested too much, so I think it's a downside I can cope with.
This is how I would use it:
```
void handleException(MyException *ex)
{
cout id() what() inner();
int t = 1;
while (innerExce
std::runtime_error to have Error-IDs, timestamps and inner exceptions. It seems to work, but are there any drawbacks?The only thing I see is the deep-copy in the copy-constructor, which is not efficient, when there are many nested exceptions. But exceptions should be rare and not be nested too much, so I think it's a downside I can cope with.
#pragma once
#include
#include
#include
#include
#include
class MyException : public std::runtime_error
{
public:
MyException(const MyException& exception)
: std::runtime_error(exception.what()),
exceptionId(exception.id()),
ts(exception.timestamp()),
innerException(NULL)
{
if (exception.inner() != NULL)
{
innerException = new MyException(*exception.inner());
}
else
{
innerException = NULL;
}
}
MyException(const std::string& _Message)
: std::runtime_error(_Message),
exceptionId(0),
innerException(NULL)
{
time(&ts);
}
MyException(const std::string& _Message, unsigned int id)
: std::runtime_error(_Message),
exceptionId(id),
innerException(NULL)
{
time(&ts);
}
MyException(const std::string& _Message, unsigned int id, MyException* innerException)
: std::runtime_error(_Message),
exceptionId(id),
innerException(new MyException(*innerException))
{
time(&ts);
}
virtual ~MyException()
{
delete innerException;
}
unsigned int id() const { return exceptionId; }
time_t timestamp() const { return ts; }
const MyException* inner() const { return innerException; }
private:
unsigned int exceptionId;
time_t ts;
const MyException* innerException;
};This is how I would use it:
```
void handleException(MyException *ex)
{
cout id() what() inner();
int t = 1;
while (innerExce
Solution
Not all compilers support
There are C++ equivalent of most C libraries that put the appropriate interface into the standard namespace.
My pet peeve (OK second after
DO NOT USE IDENTIFIERS WITH A LEADING UNDERSCORE. Even if you know the rules most people get it wrong (such as this case) as a result it leads to maintenance problems. https://stackoverflow.com/a/228797/14065 An identifier with a leading underscore and a capital letter is reserved in all scopes (i.e. the implementation is potentially going to use a macro with that name and Message seems like a prime candidate).
Your three versions of the constructor can be written as a single constructor:
A class is its own friend.
So you don't need to call getter methods to access members of the object you are copying in the copy constructor; you can just get the values. When constructing the base class (std::runtime_error) why not take advantage of its copy constructor (it may have optimizations that you are failing to take use of by using the non standard version).
The inner exception is passed around as a pointer. So we have no ownership semantics associated with it. You attempt to take ownership but fail (because you do not implement the rule of 3 (5 in C++11)).
Also because of the way you implement it you can not subclass
In the handler:
Why pass a pointer to the exception. Now you have to check for NULL. Pass it as a reference.
#pragma once#pragma onceThere are C++ equivalent of most C libraries that put the appropriate interface into the standard namespace.
#include
// Prefer
#include My pet peeve (OK second after
using namespace std;); because everybody thinks they know the rules but get it wrong all the time:_MessageDO NOT USE IDENTIFIERS WITH A LEADING UNDERSCORE. Even if you know the rules most people get it wrong (such as this case) as a result it leads to maintenance problems. https://stackoverflow.com/a/228797/14065 An identifier with a leading underscore and a capital letter is reserved in all scopes (i.e. the implementation is potentially going to use a macro with that name and Message seems like a prime candidate).
Your three versions of the constructor can be written as a single constructor:
MyException( const std::string& _Message
, unsigned int id = 0 // Use default values
, MyException* innerException = NULL) // That way you do not have
: std::runtime_error(_Message) // three versions of the
, exceptionId(id) // same constructor
, innerException(innerException)
{
time(&ts);
}A class is its own friend.
So you don't need to call getter methods to access members of the object you are copying in the copy constructor; you can just get the values. When constructing the base class (std::runtime_error) why not take advantage of its copy constructor (it may have optimizations that you are failing to take use of by using the non standard version).
MyException(const MyException& exception)
: std::runtime_error(exception)
, exceptionId(exception.exceptionId)
, ts(exception.ts)
, innerException(NULL)The inner exception is passed around as a pointer. So we have no ownership semantics associated with it. You attempt to take ownership but fail (because you do not implement the rule of 3 (5 in C++11)).
{
if (exception.inner != NULL)
{
// Slicing problem if exception.inner is derived from MyException
innerException = new MyException(exception.inner);
}
else
{
// Already set in the initializer list no need to set again.
innerException = NULL;
}
}Also because of the way you implement it you can not subclass
MyException because the copy constructor will slice your object on copy. You should probably use a smart pointer to handle ownership of the exception. Since sharing the same exception would solve most of your problems the easy way out is to use shared_ptr. Personally I would make X a std::runtime_errornotMyException` thus allowing you to chain on standard exceptions (or anything derived from them).In the handler:
void handleException(MyException *ex)Why pass a pointer to the exception. Now you have to check for NULL. Pass it as a reference.
Code Snippets
#pragma once#include <time.h>
// Prefer
#include <ctime>MyException( const std::string& _Message
, unsigned int id = 0 // Use default values
, MyException* innerException = NULL) // That way you do not have
: std::runtime_error(_Message) // three versions of the
, exceptionId(id) // same constructor
, innerException(innerException)
{
time(&ts);
}MyException(const MyException& exception)
: std::runtime_error(exception)
, exceptionId(exception.exceptionId)
, ts(exception.ts)
, innerException(NULL){
if (exception.inner != NULL)
{
// Slicing problem if exception.inner is derived from MyException
innerException = new MyException(exception.inner);
}
else
{
// Already set in the initializer list no need to set again.
innerException = NULL;
}
}Context
StackExchange Code Review Q#16357, answer score: 8
Revisions (0)
No revisions yet.