db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Knut Anders Hatlen <Knut.Hat...@Sun.COM>
Subject Re: svn commit: r542016 - in /db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4: BlobTest.java CallableStatementTest.java ClobTest.java DataSourceTest.java PreparedStatementTest.java ResultSetTest.java TestJDBC40Exception.java
Date Sun, 27 May 2007 22:30:17 GMT
Kristian Waagan <Kristian.Waagan@Sun.COM> writes:

>>> db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/CallableStatementTest.java
>>> Sun May 27 11:30:45 2007
>>> @@ -71,9 +71,9 @@
>>>       */
>>>      protected void tearDown()
>>>          throws Exception {
>>> -        if (cStmt != null && !cStmt.isClosed()) {
>>> -            cStmt.close();
>>> -        }
>>> +
>>> +        cStmt.close();
>>> +        cStmt = null;
>>>   
>>
>> Just out of curiosity:
>> At first look the existing code looks sound - calling close only if
>> the statement is not null and not already closed.
>> Why is that inadequate?
>
> A bit quick on the send button...
> I understand why you nullify the reference, what I was wondering about
> was if there was a reason for changing the if-block.
> As far as I can tell, it removes complexity, but increases the chance
> of a NPE. Unless there is some other reason I don's see?

Since none of the test cases in that test closes cStmt or sets it to
null, I didn't see any reason to check for it. As to the increased
chance of NPE, since the variable is initialized in setUp() and
shouldn't be changed afterwards, a NPE would (correctly) have been
thrown much earlier if prepareCall() had returned null. It seems like
it's the common practice in the tearDown() methods just to call close
with no checks, but feel free to change it back.

-- 
Knut Anders

Mime
View raw message