Hi, Tomohito, thanks for your quick review of this patch.
TomohitoNakayama wrote:
Hello.
I have reviewed the patch.
point-1:
In the code of toString() in
"java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java",
there exists four local variable which are declared and not used.
They are dbname, sessid, drdaid, txid .
I think they should be used or does not exist.
The local variables are used as part of the assignment statment for
connString. I do this for readability, otherwise the assignment of
connString becomes fairly long and involved. I suppose not using local
variables would save /some/ space, but I think it's important for
readability. And note they are only created one time, the first time
toString() is called. After that the entire string is cached in a
single variable and there is no cost in terms of time or space.
point-2:
In the comment of toString() in
"java/engine/org/apache/derby/iapi/jdbc/BrokeredConnection.java",
description of unique id and lifetime of the connection and such
was deleted.
I think they should be described in somewhere else and remained in
the source codes.
BrokeredConnection no longer has a unique id. In the new version of
BrokeredConnection.toString(), all that's done is to print out the
classname and hashcode and the string representation of the underlying
connection. I didn't feel this needed much of a comment.
point-3:
In
"java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/checkDataSource.java",
there exists newly added instance method such as checkStringFormat,
checkStringFormat, checkStringFormat, checkStringPrefix.
I think static method is prefered because it does not have scope to
instance variables , if not much difficult.
I can do this, but can you explain further why static is preferred over
non-static? Multiple instances of this class aren't being created; is
there another reason why static is better?
Thanks,
David
Best regards.
/*
Tomohito Nakayama
[EMAIL PROTECTED]
[EMAIL PROTECTED]
[EMAIL PROTECTED]
Naka
http://www5.ocn.ne.jp/~tomohito/TopPage.html
*/
----- Original Message ----- From: "David Van Couvering (JIRA)"
<[email protected]>
To: <[email protected]>
Sent: Friday, July 29, 2005 3:02 AM
Subject: [jira] Updated: (DERBY-412) Connection toString should show
type information and the meaning of the identifier that it prints
[ http://issues.apache.org/jira/browse/DERBY-412?page=all ]
David Van Couvering updated DERBY-412:
--------------------------------------
Attachment: DERBY-412.diff
This patch contains the changes based on Kathey's feedback. In
particular, I fixed BrokeredConnection. I also verify that the
format of the connection strings is correct in the test code.
derbyall passes all tests
svn status output:
M java\engine\org\apache\derby\impl\jdbc\EmbedConnection.java
M java\engine\org\apache\derby\iapi\jdbc\BrokeredConnection.java
M java\engine\org\apache\derby\jdbc\EmbedPooledConnection.java
M
java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\build.xml
M
java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\checkDataSource.java
Connection toString should show type information and the meaning of
the identifier that it prints
--------------------------------------------------------------------------------------------------
Key: DERBY-412
URL: http://issues.apache.org/jira/browse/DERBY-412
Project: Derby
Type: Bug
Versions: 10.1.1.0, 10.2.0.0
Reporter: Kathey Marsden
Assignee: David Van Couvering
Fix For: 10.2.0.0
Attachments: DERBY-412.diff
After the change for DERBY-243 the connection toString() output is
an integer which correspond to SESSIONID. The output should
identify the type and also the meaning of the identifier that it
prints. Perhaps a format that appends the default toString output
with the sessionid information as it prints in the derby.log would
be more informative.
[EMAIL PROTECTED] (SESSONID = 2)
Ultimately this could be expanded to included other diagnostic
information e.g
[EMAIL PROTECTED] (XID = 132),
(SESSIONID = 5), (DATABASE = wombat), (DRDAID =
NF000001.H324-940125304405039114{7})
--
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
--
No virus found in this incoming message.
Checked by AVG Anti-Virus.
Version: 7.0.338 / Virus Database: 267.9.6/59 - Release Date: 2005/07/27
begin:vcard
fn:David Van Couvering
n:Van Couvering;David
org:Sun Microsystems, Inc.;Database Technology Group
email;internet:[EMAIL PROTECTED]
title:Senior Staff Software Engineer
tel;work:510-550-6819
tel;cell:510-684-7281
x-mozilla-html:TRUE
version:2.1
end:vcard
