Hello.
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.
Assighment statement for connString was not using local variables...
+ connString =
+ this.getClass().getName() + "@" + this.hashCode() + " " +
+ lcc.xidStr +
+ lcc.getTransactionExecute().getTransactionIdString() +
+ "), " +
+ lcc.lccStr +
+ Integer.toString(lcc.getInstanceNumber()) + "), " +
+ lcc.dbnameStr + lcc.getDbname() + "), " +
+ lcc.drdaStr + lcc.getDrdaID() + ") ";
I think you should use local variable in this assginment statement.
(or declaring new method for this assignment statement and save local variable
in toString() method)
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.
I found next mail.
http://article.gmane.org/gmane.comp.apache.db.derby.devel/6720
I understand BrokeredConnection no longer has a unique id.
Then this point was solved.
I think this part have no problem too.
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?
Instance method , which have scope to instance variable, would change state of
instance.
Then it is more toroublesome than class method (static method) .
Futhermore, if here comes needs to use these method in another classes,
it is more easy for us to move class method to another class than to move
instance method.
(And constructer of this class was declared as public.
Then I think it is not guranteed that multiple instance of this class is
created.
It is out of imagine multiple instance of this class was created in other part,
but trouble often happens in the situation that is out of imagine .....)
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" <[EMAIL PROTECTED]>
To: "Derby Development" <[email protected]>
Sent: Saturday, July 30, 2005 3:53 AM
Subject: Re: [jira] Updated: (DERBY-412) Connection toString should show type information and the meaning of the identifier that it
prints
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
--------------------------------------------------------------------------------
No virus found in this incoming message.
Checked by AVG Anti-Virus.
Version: 7.0.338 / Virus Database: 267.9.7/60 - Release Date: 2005/07/28
--
No virus found in this outgoing message.
Checked by AVG Anti-Virus.
Version: 7.0.338 / Virus Database: 267.9.7/60 - Release Date: 2005/07/28
