Sounds like the proposal currently stands to avoid the DiamondOfDeath or TriangleOfLove that multiple inheritance brings us.... and just have the base Exception class inherit std::exception and extend Exception class as appropriate within the library.
+1 and thanks for the code examples, very illistrative! +1 to Boost stack trace as well... On Thu, Sep 14, 2017 at 10:10 AM, Jacob Barrett <jbarr...@pivotal.io> wrote: > The problem stems from the fact that none of the std exceptions virtually > inherit from std::exception so you end up in the inheritance triangle of > love. I say we avoid the multiple inheritance issues with exceptions by > avoiding multiple inheritance altogether in exceptions. See this example. > http://coliru.stacked-crooked.com/a/2e32e7777b021e85 > > Basically all of our exceptions extend our Exception class which extends > std::exception. None extend any of the other std exceptions, like > bad_alloc, etc. The downside is you can't catch any std exception other > than std::exception but the upside is that this actually works. There were > also very few exceptions that actually could extend any std exceptions > beyond std::exception. > > -Jake > > > On Wed, Sep 13, 2017 at 10:52 PM Jacob Barrett <jbarr...@pivotal.io> > wrote: > > > Let me see if I can considerate the consensus here: > > > > As reasonably possible throw library specific exceptions from all public > > methods. > > > > As reasonably possible those exceptions should extend C++11 standard > > library exceptions. > > > > All exceptions should extend less specific but logically relevant base > > exceptions. > > > > A few quick examples: > > namespace apache { > > namespace geode { > > namespace client { > > > > class Exception : public std::exception {...}; > > > > class IllegalArgumentException : public Exception, public > > std::invalid_argument {...}; > > > > class TransactionException : public Exception {...}; > > > > class RollbackException : public TransactionException {...}; > > > > } > > } > > } > > > > Additionally, investigate using Boost Stack Track library for providing > > stack context in exceptions, otherwise dump the current stack tracing > > feature that is incomplete and very platform specific. > > > > Does anyone have a different understanding of the consensus? > > > > > > I found some problems with this model. The IllegalArgumentException would > > not be caught in a catch (const std::exception& e) statement do to > > multiple inheritance. Another nasty is std::exception doesn't have a > > constructor to populate the what() value. Some examples of this problem > > can be seen here: > > http://coliru.stacked-crooked.com/a/5b6e34c7ea020fc8 > > > > > > -Jake > > > > > > > > > > On Wed, Sep 13, 2017 at 4:37 PM Mark Hanson <mhan...@pivotal.io> wrote: > > > >> I think that it would be best to abide by using the std::exception as > the > >> base. I think it brings with it all the language support and > flexibility. > >> There are a few penalties obviously to using std::exception as a base, > but > >> I think they are sufficiently offset by using a standard. As far as the > >> number of exceptions, I believe in the philosophy of using as few as > >> possible and using inheritance to drive specificity. The benefit is that > >> the code can be as simple or as complex as it can be without unnecessary > >> overhead e.g error => network error => tcp error. So, I may just care > there > >> is a network error and the being able to tell if something derives from > >> network error is perfect. > >> > >> Thanks, > >> Mark > >> > >> > >> > On Sep 8, 2017, at 1:35 PM, Ernest Burghardt <eburgha...@pivotal.io> > >> wrote: > >> > > >> > if we continue the merging of Jake <> Sarge comments we might find > that > >> > std::exception(s) is sufficient if the many name exceptions that > >> pertain to > >> > the Library are all handled in a similar manner and only have unique > >> names > >> > for human readability, but not a functional consideration... > >> > > >> > EB > >> > > >> > On Fri, Sep 8, 2017 at 8:52 AM, Michael William Dodge < > >> mdo...@pivotal.io> > >> > wrote: > >> > > >> >> I subscribe to Josh Gray's philosophy of only having another > exception > >> >> class if there is something different to be done when it's caught. > For > >> >> example, if the caller would do the exact same thing for > >> >> NoPermissionsException and DiskFullException, just use an IOException > >> and > >> >> be done with it. I also subscribe to the philosophy that a library > >> have its > >> >> own exception hierarchy (possibly with a single class), which I think > >> >> meshes with Jake's "exceptions exiting a library to be reasonably > >> limited > >> >> to exceptions generated by and relating to that library". > >> >> > >> >> Sarge > >> >> > >> >>> On 8 Sep, 2017, at 07:19, Jacob Barrett <jbarr...@pivotal.io> > wrote: > >> >>> > >> >>> Sorry for jumping on this thread so late. This is an important issue > >> we > >> >>> need to address. > >> >>> > >> >>> On Thu, Aug 17, 2017 at 11:57 AM David Kimura <dkim...@pivotal.io> > >> >> wrote: > >> >>> > >> >>>> Using exceptions seems contrary to the Google C++ Style Guide we > >> >> adopted, > >> >>>> which states: *"do not use C++ exceptions"* [3 > >> >>>> <https://google.github.io/styleguide/cppguide.html#Exceptions>]. > >> Here > >> >> is > >> >>>> a > >> >>>> link [4 <https://www.youtube.com/watch?v=NOCElcMcFik#t=30m29s>] > to a > >> >>>> cppcon > >> >>>> talk defending their decision. Does it make sense to enforce this > >> rule > >> >> on > >> >>>> our code base? > >> >>>> > >> >>> > >> >>> I don't agree with this approach, as I always tend towards > >> >>> progress/modernization, but it was their choice to remain consistent > >> >> across > >> >>> their code base. I would say if we made the same decision solely on > >> >>> consistency then we would have our own exceptions derived from a > base > >> >>> exception class. This is consistent with our Java and .NET as well > as > >> >>> current C++ clients. > >> >>> > >> >>> > >> >>>> If we decide to knowingly ignore this rule, would we want to > >> continue to > >> >>>> propagate custom exceptions or switch to standard exceptions? At > the > >> >> very > >> >>>> least, it seems to me that our custom exceptions should derive from > >> >> their > >> >>>> most closely matching standard exception counterparts. Thoughts? > >> >>>> > >> >>> > >> >>> I agree with your recommendation of using our exceptions but extend > >> the > >> >>> most appropriate C++11 exceptions where applicable. I think it is > good > >> >> form > >> >>> for exceptions exiting a library to be reasonably limited to > >> exceptions > >> >>> generated by and relating to that library. > >> >>> > >> >>> Our current Exception class also brings with it stack tracing, > >> although > >> >>> poorly supported on some platforms and disabled by default. Boost > just > >> >>> recently graduated a stack track library that is supported on many > >> >>> compilers and platforms. It may be worth integrating into our > >> exceptions > >> >> as > >> >>> a replacement for the current stack tracing code. > >> >>> > >> >>> -Jake > >> >> > >> >> > >> > >> >
