db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David Van Couvering (JIRA)" <derby-...@db.apache.org>
Subject [jira] Commented: (DERBY-336) The wrong overload of StandardException::newException() is used in some cases
Date Tue, 28 Jun 2005 06:54:57 GMT
    [ 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


Mime
View raw message