Attachment 'exception_discussion.txt'

Download

   1 Malte Helmert
   2 I think in C++ it's considered common practice not to try to fit into the
   3 stdlib's exception hierarchy. I would just derived from std::exception.
   4 
   5 Malte Helmert
   6 Applies to both classes, of course.
   7 
   8 Jendrik Seipp
   9 In total, I think it applies to four classes. I'm happy to let them inherit from
  10 std::exception, but this would mean adding more boilerplate code, I think. You
  11 can see the difference by looking at the last commit in this branch
  12 (https://bitbucket.org/jendrikseipp/downward/commits/f07cfcc51640bf67c7614767f6b0405e21a8beb9),
  13 which changed the parent class from std::exception to std::runtime_error. A
  14 quick Google search brought up no examples of people advocating against
  15 inheriting from std::runtime_error. I only found two sites that say it’s fine:
  16 https://stackoverflow.com/questions/1569726/difference-stdruntime-error-vs-stdexception,
  17 https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#e14-use-purpose-designed-user-defined-types-as-exceptions-not-built-in-types
  18 .
  19 
  20 Malte Helmert
  21 I found the same links (before reading your message), but I don't read them as
  22 endorsements of deriving from any specific base class. The stackoverflow link
  23 describes basic C++ features and how the two classes differ in behaviour; it is
  24 not a style recommendation. It is written for a beginner audience, so it is
  25 about what the classes do and how they differ in usage and interface, not
  26 advanced considerations. I am not sure you understood the point that the C++
  27 core guidelines article wants to make. It basically says: “never do throw 7,
  28 never do throw runtime_error("foo"); always use your own exception classes.
  29 Using throw and catch with a standard library type may deserve a warning. Throw
  30 and catch of a type derived from a standard library exception type is obviously
  31 OK." I think you misinterpret the latter sentence as an endorsement about which
  32 classes are the best ones to derive from; it merely means that the
  33 prohibition/warning against catch(std::exception) or catch(std::runtime_error)
  34 is not meant to apply to derived classes of our own, so catch(MyError) where
  35 MyError is derived from one of the standard library classes is OK. From this it
  36 doesn’t follow that any class works equally well as a base class in any context;
  37 that is a separate consideration.
  38 
  39 The main problems I see with deriving from std::runtime_error here are: it's
  40 inconsistent with the rest of the codebase, whose existing exceptions derive
  41 directly from std::exception it implies that we're following the categorization
  42 of the standard library, in which case I think we need to think very carefully
  43 for each of our exception types whether they should derive from logic_error, one
  44 of its subclasses, runtime_error, one of its subclasses, or bad_cast (just
  45 naming those that are more likely to apply, but I may have missed some).
  46 
  47 Basically, the question is whether we want “our” exceptions to be interwoven
  48 with the ones of the standard library or not. All the larger C++ codebases I’ve
  49 seen have gone for the latter because the standard library’s exceptions are not
  50 necessarily designed with the idea of extensibility as a “user hierarchy” where
  51 everything can find its natural place in mind. The existing exceptions are there
  52 because the standard library needs to throw them, not because someone sat down
  53 and designed a general-purpose exception hierarchy.
  54 
  55 Most major codebases I've seen have their own "root exception" class that they
  56 use, sometimes derived from std::exception, sometimes not. (Note that the core
  57 guidelines do not actually recommend deriving from the std::exception hierarchy.
  58 Neither do they recommend against it.) I’m not actually sure what the best way
  59 to proceed here is, but no matter what we do, what I don’t like is different
  60 parts of the codebase following different conventions.
  61 
  62 Jendrik Seipp
  63 Thanks for elaborating! I have no strong opinions for the design of our
  64 exceptions, so I’m happy to follow your preference. Let me make a simple
  65 proposal, which also helps with the std::string vs. cout issue we’re discussing
  66 in the other thread for this pull request:
  67 
  68     # in utils/exceptions.h
  69 
  70     namespace utils {
  71     class Exception : public std::exception {
  72     public:
  73         virtual void print() = 0;
  74     }
  75     }
  76 
  77 Then all our exception classes could derive from this class. I'm not sure
  78 whether the class should inherit from std::exception and if yes how it should
  79 override the what method.
  80 
  81 Malte Helmert
  82 Sounds good to me! The main thing that we should care about here is consistency,
  83 and this would establish consistency, while also being straightforward. I'm not
  84 sure if it would be better to derive from std::exception or not, so perhaps as a
  85 tie-breaker we should consider that we don't know what to do with what() and
  86 therefore not derive from std::exception.
  87 
  88 I’m not sure how the exception we use for the timeout in the hill-climbing code
  89 fits in, though. It’s an internal implementation detail, and it doesn’t really
  90 need an error message. Perhaps it would be best for that one to be standalone
  91 and not derive from anything, with a comment explaining why. It’s not really a
  92 “normal” exception but somewhere between a regular exception and something used
  93 for flow control (which is discouraged, but in this case I think it’s
  94 acceptable, as it’s somewhere in the middle between that and a “usual” use of an
  95 exception).

Attached Files

To refer to attachments on a page, use attachment:filename, as shown below in the list of files. Do NOT use the URL of the [get] link, since this is subject to change and can break easily.
  • [get | view] (2020-07-08 16:48:28, 4.9 KB) [[attachment:const_discussion.txt]]
  • [get | view] (2020-07-08 16:56:29, 5.3 KB) [[attachment:exception_discussion.txt]]
 All files | Selected Files: delete move to page copy to page

You are not allowed to attach a file to this page.