geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Darrel Schneider (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (GEODE-2375) GemFireException should not inherit from RuntimeException
Date Fri, 10 Feb 2017 23:58:41 GMT

    [ https://issues.apache.org/jira/browse/GEODE-2375?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15862026#comment-15862026
] 

Darrel Schneider commented on GEODE-2375:
-----------------------------------------

As the javadocs on GemFireException state it should have been named "GemFireRuntimeException".
So I think it is wrong to say it should not inherit from RuntimeException. Instead you should
say: "it should be named GemFireRuntimeException".

We also have a GemFireCheckedException (whose javadocs say it should have been named GemFireException).

It is not clear to me why you say: "which means that the majority of exceptions in Geode are
unchecked". Any of our exception subclasses are free to extend either GemFireException or
GemFireCheckedException. If they want to be a checked exception they are not forced to extend
GemFireException. Exceptions are not added that often but when they are the author should
consider if it should be a checked one or not and extend the correct class.

The reason these two exception classes did not have their name changed before was because
of backwards compatibility. You would want to be careful not to break old code when trying
to convert old exceptions to GeodeCheckedException and GeodeUncheckedException.

> GemFireException should not inherit from RuntimeException
> ---------------------------------------------------------
>
>                 Key: GEODE-2375
>                 URL: https://issues.apache.org/jira/browse/GEODE-2375
>             Project: Geode
>          Issue Type: Bug
>          Components: core, general
>            Reporter: Galen O'Sullivan
>
> {{GemFireException}} inherits from {{RuntimeException}}, which means that the majority
of exceptions in Geode are unchecked. This means that we don't have the type system helping
us to check potential failure conditions of our code, and it's not clear which functions may
throw exceptions as a part of their nomal failure modes -- for example, {{ReplyException}}
has a {{handleAsUnexpected}} method that seems to indicate that a normal {{ReplyException}}
is not unexpected -- but that's not what the type inheritance says. {{GemFireException}} accounts
for most of the exceptions in the codebase.
> Even if we were to convert most of the existing instances of {{GemFireException}} to
{{GemFireRuntimeException}}, developers (especially new ones) would still be tempted to use
{{GemFireException}} for new exceptions.
> Perhaps the best way to solve this (if we want all our exceptions to inherit from a central
exception type, which I'm not entirely sold on) would be to create a new {{GeodeUncheckedException}}
and {{GeodeCheckedException}}, and deprecate both kinds of {{GemFireException}}? Then we could
convert old exceptions as time permits.
> There's a significant amount of work involved here whatever way we decide to change it.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Mime
View raw message