HiveBrain v1.2.0
Get Started
← Back to all entries
debugcppMinor

Self-implemented C++ exception class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementedclassexceptionself

Problem

I wrote my own exception class, deriving from 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 #pragma once

#pragma once


There 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:

_Message


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:

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.