[
https://issues.apache.org/jira/browse/DRILL-2626?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Daniel Barclay (Drill) updated DRILL-2626:
------------------------------------------
Description:
It seems that class org.apache.drill.common.StackTrace needlessly duplicates
code that's already in the JDK.
In particular, it has code to format the stack trace. That seems at least
mostly redundant with the formatting code already in java.lang.Throwable.
StackTrace does have a comment about eliminating the StackTrace constructor
from the stack trace. However, StackTrace does _not_ actuallly eliminate its
contructor from the stack trace (e.g., its stack traces start with
"org.apache.drill.common.StackTrace.<init>:...").
Should StackTrace be implemented by simply subclassing Throwable?
That would eliminate StackTrace's current formatting code (which would also
eliminate the difference between StackTrace's format and the standard format).
That should also eliminate having the StackTrace constructor's stack frame show
up in the stack trace. (Throwable's constructor/fillInStackTrace already
handles that.)
(Having "StackTrace extends Throwable" isn't ideal, since StackTrace is not
intended to be a kind of exception, but that would probably be better than the
current form, given the bugs StackTrace has/has had (DRILL-2624, DRILL-2625).
That non-ideal subclassing could be eliminated by having a member variable of
type Throwable that is constructed during StackTrace's construction, although
that would either cause the StackTrace constructor to re-appear in the stack
trace or require a non-trivial workaround to re-eliminate it.
Perhaps client code should simply use "new Throwable()" to capture the stack
trace and a static methods on a utility class to format the stack trace into a
String.)
was:
It seems that class org.apache.drill.common.StackTrace needlessly duplicates
code that's already in the JDK.
In particular, it has code to format the stack trace. That seems at least
mostly redundant with the formatting code already in java.lang.Throwable.
StackTrace does have a comment about eliminating the StackTrace constructor
from the stack trace. However, StackTrace does _not_ actuallly eliminate its
contructor from the stack trace (e.g., its stack traces start with
"org.apache.drill.common.StackTrace.<init>:...").
Should StackTrace be implemented by simply subclassing Throwable?
That would eliminate StackTrace's current formatting code (which would also
eliminate the difference between StackTrace's format and the standard format).
That should also eliminate having the StackTrace constructor's stack frame show
up in the stack trace. (Throwable's constructor/fillInStackTrace already
handles that.)
(Having "StackTrace extends Throwable" isn't ideal, since StackTrace is not
intended to be a kind of exception, but that would probably be better than the
current form, given the bugs StackTrace has/has had (DRILL-xxxxx, DRILL-xxxx).
That non-ideal subclassing could be eliminated by having a member variable of
type Throwable that is constructed during StackTrace's construction, although
that would either cause the StackTrace constructor to re-appear in the stack
trace or require a non-trivial workaround to re-eliminate it.
Perhaps client code should simply use "new Throwable()" to capture the stack
trace and a static methods on a utility class to format the stack trace into a
String.)
> org.apache.drill.common.StackTrace seems to have duplicate code; should we
> re-use Throwable's code?
> ---------------------------------------------------------------------------------------------------
>
> Key: DRILL-2626
> URL: https://issues.apache.org/jira/browse/DRILL-2626
> Project: Apache Drill
> Issue Type: Bug
> Reporter: Daniel Barclay (Drill)
>
> It seems that class org.apache.drill.common.StackTrace needlessly duplicates
> code that's already in the JDK.
> In particular, it has code to format the stack trace. That seems at least
> mostly redundant with the formatting code already in java.lang.Throwable.
> StackTrace does have a comment about eliminating the StackTrace constructor
> from the stack trace. However, StackTrace does _not_ actuallly eliminate its
> contructor from the stack trace (e.g., its stack traces start with
> "org.apache.drill.common.StackTrace.<init>:...").
> Should StackTrace be implemented by simply subclassing Throwable?
> That would eliminate StackTrace's current formatting code (which would also
> eliminate the difference between StackTrace's format and the standard format).
> That should also eliminate having the StackTrace constructor's stack frame
> show up in the stack trace. (Throwable's constructor/fillInStackTrace
> already handles that.)
> (Having "StackTrace extends Throwable" isn't ideal, since StackTrace is not
> intended to be a kind of exception, but that would probably be better than
> the current form, given the bugs StackTrace has/has had (DRILL-2624,
> DRILL-2625).
> That non-ideal subclassing could be eliminated by having a member variable of
> type Throwable that is constructed during StackTrace's construction, although
> that would either cause the StackTrace constructor to re-appear in the stack
> trace or require a non-trivial workaround to re-eliminate it.
> Perhaps client code should simply use "new Throwable()" to capture the stack
> trace and a static methods on a utility class to format the stack trace into
> a String.)
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
