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.You are not allowed to attach a file to this page.