db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dyre Tjeldvoll (JIRA)" <derby-...@db.apache.org>
Subject [jira] Commented: (DERBY-336) The wrong overload of StandardException::newException() is used in some cases
Date Mon, 27 Jun 2005 13:08:57 GMT
    [ http://issues.apache.org/jira/browse/DERBY-336?page=comments#action_12314532 ] 

Dyre Tjeldvoll commented on DERBY-336:
--------------------------------------

Thanks for your comments, David!

I see what you are saying about FILE_CANNOT_REMOVE_FILE, but look at how it is used other
places in the code:

./java/engine/org/apache/derby/impl/store/raw/data/RFResource.java:261:
           SQLState.FILE_CANNOT_REMOVE_FILE, fileToGo);
./java/engine/org/apache/derby/impl/store/raw/data/RAFContainer.java:1077:
               newException(SQLState.FILE_CANNOT_REMOVE_FILE, se, file,
./java/engine/org/apache/derby/impl/store/raw/data/RAFContainer.java:1102:
                   SQLState.FILE_CANNOT_REMOVE_FILE, ioe2, file, ioe);
./java/engine/org/apache/derby/impl/store/raw/data/RAFContainer.java:1107:
                   SQLState.FILE_CANNOT_REMOVE_FILE, se, file, stub);
./java/engine/org/apache/derby/iapi/reference/SQLState.java:489:        String FILE_CANNOT_REMOVE_FILE
                             = "XSDF4.S";

As you can see, this error message is used both with one and TWO nested exceptions (in much
the same way as FILE_CREATE_NO_CLEANUP), 
so you cannot remove the additional parameter from the message string. I agree that you can
choose to supply the same exception both as the nested exception, and as a parameter, but
I doubt that this is why the error message is written the way it is... Also, notice the first
use of this message, where NO nested exception is supplied! 
---
I think you are correct about using a string argument. However, I was cautioned about converting
arguments to string before it is necessary, as this could lead to performance problems,  (if
the exception is caught, the conversion to string is unnecessary). But in this particular
case I doubt that performance is relevant, so I'll consider changing it... 
---
I understand your concerns about the dummy overloads, - it is a non-standard and non-intuitive
construct. I was not sure if I should include them in the patch or just use them locally to
find the StandardException usages that needed to be fixed. In the end, I decided to include
them since it is easy to make another such mistake later, (and you will not be warned about
it).

> 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