[
http://issues.apache.org/jira/browse/DERBY-336?page=comments#action_12314591 ]
David Van Couvering commented on DERBY-336:
-------------------------------------------
Hi, Dyre, your responses seem good.
There is still something funny about FILE_CANNOT_REMOVE_FILE. If there is a
case where the second parameter is not even used, I would prefer that that
usage be split into a separate message, rather than inserting a dummy string.
Regarding the dummy overloads: the reason mistakes have been made and you have
had to do this is because the semantics of the newException are not clear: when
you want to chain an exception, where does it go? How does one distinguish a
chained exception from a parameter to the message string? It's particularly
confusing because in some cases you want an exception in both places (sometimes
the same exception, sometimes different exceptions).
I think in the name of convenience (e.g. only writing one line to throw the
exception) we have created semantic muddiness, and it is now forcing us to do
unnatural acts to get the compiler to catch improper usage. I
I would like to suggest opening a JIRA item to fix this so the semantics are
clear and the compiler can catch mistakes the way it was intended to do so --
with strong typing.
Perhaps this could be done as part of the larger task of migrating to J2SE 1.4
chained exceptions... I can envision only allow message string arguments to
newException(), and chaining exceptions be allowed only by calling a separate
method on the exception class, e.g.:
catch ( IOException ioe ) {
try { somethingProtected() }
catch ( SecurityException se ) {
StandardException stde = StandardException.newException(msgid, arg1,
arg2, ioe);
se.setCause(ioe)
stde.setCause(se);
throw stde;
}
}
In terms of this patch, however, I think what you have works pretty well and
prevents further misuse, as long as it's commented extremely well why we're
doing this, because it is very odd.
> The wrong overload of StandardException::newException() is used in some cases
> -----------------------------------------------------------------------------
>
> Key: DERBY-336
> URL: http://issues.apache.org/jira/browse/DERBY-336
> Project: Derby
> Type: Bug
> Environment: Any
> Reporter: Dyre Tjeldvoll
> Assignee: Dyre Tjeldvoll
> Priority: Trivial
> Attachments: derby-336.diff, derby-336.stat, derbyall_report.txt
>
> When looking at DERBY-128 it became clear that the wrong overload of
> StandardException::newException() was used when reporting
> SQLState.SERVICE_DIRECTORY_CREATE_ERROR. The message string only takes one
> parameter so only one additional parameter (other than Throwable) should be
> used:
> PersistentServiceImpl.java:676
> throw
> StandardException.newException(SQLState.SERVICE_DIRECTORY_CREATE_ERROR,
>
> serviceDirectory, null);
> // Calls StandardException.newException(String, Object, Object)
> // Should call StandardException.newException(String, Object)? Or
> StandardException.newException(String, Throwable, Object)? With the
> IOException as
> // Throwable?
> PersistentServiceImpl.java:692
> throw
> StandardException.newException(SQLState.SERVICE_DIRECTORY_CREATE_ERROR, name,
> t);
> // Calls StandardException.newException(String, Object, Object)
> // Should call StandardException.newException(String, Throwable, Object)?
> BaseDataFileFactory.java:279
> throw StandardException.newException(
> SQLState.SERVICE_DIRECTORY_CREATE_ERROR, dataDirectory, ioe);
> // Calls StandardException.newException(String, Object, Object)
> // Should call StandardException.newException(String, Throwable, Object)?
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
