You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by David Van Couvering <Da...@Sun.COM> on 2005/08/01 19:03:00 UTC
Re: [jira] Updated: (DERBY-412) Connection toString should show type
information and the meaning of the identifier that it prints
Hi, Tomohito, my responses inline
TomohitoNakayama wrote:
> 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)
Yep, you're right, I agree. Thanks.
>
>
>>> 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.
>
OK
>
>>> 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 .....)
OK, I can change this
>
>
> Best regards.
>
>
> /*
>
> Tomohito Nakayama
> tomonaka@basil.ocn.ne.jp
> tomohito@rose.zero.ad.jp
> tmnk@apache.org
>
> Naka
> http://www5.ocn.ne.jp/~tomohito/TopPage.html
>
> */
> ----- Original Message ----- From: "David Van Couvering"
> <Da...@Sun.COM>
> To: "Derby Development" <de...@db.apache.org>
> 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
>>> tomonaka@basil.ocn.ne.jp
>>> tomohito@rose.zero.ad.jp
>>> tmnk@apache.org
>>>
>>> Naka
>>> http://www5.ocn.ne.jp/~tomohito/TopPage.html
>>>
>>> */
>>> ----- Original Message ----- From: "David Van Couvering (JIRA)"
>>> <de...@db.apache.org>
>>> To: <de...@db.apache.org>
>>> 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.
>>>>> org.apache.derby.impl.jdbc.EmbedConnection@efd552 (SESSONID = 2)
>>>>> Ultimately this could be expanded to included other diagnostic
>>>>> information e.g
>>>>> org.apache.derby.impl.jdbc.EmbedConnection@efd552 (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
>
>
>
Re: [jira] Updated: (DERBY-412) Connection toString should show type information and the meaning of the identifier that it prints
Posted by TomohitoNakayama <to...@basil.ocn.ne.jp>.
Hello.
I think we have agreed now :)
I wait for your new patch , and will try test on my site for confirm.
Best regards.
/*
Tomohito Nakayama
tomonaka@basil.ocn.ne.jp
tomohito@rose.zero.ad.jp
tmnk@apache.org
Naka
http://www5.ocn.ne.jp/~tomohito/TopPage.html
*/
----- Original Message -----
From: "David Van Couvering" <Da...@Sun.COM>
To: "Derby Development" <de...@db.apache.org>
Sent: Tuesday, August 02, 2005 2:03 AM
Subject: Re: [jira] Updated: (DERBY-412) Connection toString should show type information and the meaning of the identifier that it
prints
> Hi, Tomohito, my responses inline
>
> TomohitoNakayama wrote:
>
>> 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)
>
>
> Yep, you're right, I agree. Thanks.
>
>>
>>
>>>> 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.
>>
> OK
>
>>
>>>> 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 .....)
>
> OK, I can change this
>
>>
>>
>> Best regards.
>>
>>
>> /*
>>
>> Tomohito Nakayama
>> tomonaka@basil.ocn.ne.jp
>> tomohito@rose.zero.ad.jp
>> tmnk@apache.org
>>
>> Naka
>> http://www5.ocn.ne.jp/~tomohito/TopPage.html
>>
>> */
>> ----- Original Message ----- From: "David Van Couvering"
>> <Da...@Sun.COM>
>> To: "Derby Development" <de...@db.apache.org>
>> 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
>>>> tomonaka@basil.ocn.ne.jp
>>>> tomohito@rose.zero.ad.jp
>>>> tmnk@apache.org
>>>>
>>>> Naka
>>>> http://www5.ocn.ne.jp/~tomohito/TopPage.html
>>>>
>>>> */
>>>> ----- Original Message ----- From: "David Van Couvering (JIRA)"
>>>> <de...@db.apache.org>
>>>> To: <de...@db.apache.org>
>>>> 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.
>>>>>> org.apache.derby.impl.jdbc.EmbedConnection@efd552 (SESSONID = 2)
>>>>>> Ultimately this could be expanded to included other diagnostic
>>>>>> information e.g
>>>>>> org.apache.derby.impl.jdbc.EmbedConnection@efd552 (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 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