3
\$\begingroup\$

I have a custom exception hierarchy in C++ to detect unexpected cases in a library I am implementing. The base exception class inherits from 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 <exception>

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:
};

#endif

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

#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: <const char*> 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).

Does this implementation make sense? How can the above design be improved?

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 have read that such errors are better handled via assert statements because they don't represent "exceptional" cases. Could anyone please comment on this?

\$\endgroup\$
7
\$\begingroup\$

You should probably inherit from 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: <const char*> 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?

\$\endgroup\$
  • \$\begingroup\$ Just as a note, those virtual destructors (both definition and implementation) are needed otherwise you'll get a looser throw specifier error when you compile, see: stackoverflow.com/questions/22698653/… \$\endgroup\$ – ifma Apr 17 '17 at 14:10
  • 1
    \$\begingroup\$ @ifma: I don't believe that is true and that's not how I read that linked article. Since C++11 the default for a destructor is noexcept(true) which is the same as noxexcept which means it has the same specifications as std::exception::~exception(). Note throws() has been deprecated. The conclusion of the only answer seems to be the OP is not using an up-to spec compiler (ie C++03 compiler). \$\endgroup\$ – Martin York Apr 17 '17 at 16:02
  • \$\begingroup\$ @MartinYork You are amazing. How can you know everything? I'm seeing you in every single Code Review question I visit. : D It is just admirable. \$\endgroup\$ – Nikos Oct 18 '18 at 4:35
  • 1
    \$\begingroup\$ @Nik-Lz: There is actually an issue in that. There are actually very few people reviewing C++ questions. Top C++ Reviewers as you can see it drops off quickly. \$\endgroup\$ – Martin York Oct 18 '18 at 4:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.