db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oystein.Grov...@Sun.COM (Øystein Grøvlen)
Subject Re: [jira] Updated: (DERBY-243) connection toString should uniquely identify the connection
Date Mon, 06 Jun 2005 09:24:52 GMT
>>>>> "DVC" == David Van Couvering <David.Vancouvering@Sun.COM> writes:

    DVC> Hi, Kathey.  Nobody has tested this patch.  I think it's ready to go, 
    DVC> but the last patch I did with a full derbyall that then resulted in a 
    DVC> bunch of failures for Dan indicates we really need somebody else to 
    DVC> review and test it.  So far no volunteers.


I have looked at the patch and it basically looks good.  I have a few
comments/questions:

    - BrokeredConnection.toString(): Are you guranteed that
      getRealConnection never returns null?
    - checkDataSource.checkNesConn(): Why have you changed it to
      "throws Exception" instead of SQLException?  This disables the
      whole idea of advertising checked exceptions.  (Same class as
      "import java.*").  I also see that your new methods advertises
      that they "throws Exceptions".  I think you should be more
      specific.
    - While in the checkDataSource test you close the extra
      connections you create, the dataSourcePermissions test does not
      do so?  Why is this less necessary in this test?
    - Why do you want to write something in
      TestUtil.checkUniqueToString?  It just creates a lot of hazzle
      to have to sed this from the tests.  If I were to write a test
      that needed to check that strings were not equal, I think I
      would make my on method instead of reusing this method in order
      not to have to worry about "sedding".  (I also think
      ...NotEqual.. is a more descriptive name than ...Unique... for
      this method.)
    - Is it OK for the toString method to not write the class name?  I
      have not investigated how this is normally done in Derby, but I
      think many would expect toString() to print the type of an
      object.
    - The tests now test that the Ids of two connections are not
      equal.  However, for all possible bugs giving a non-unique id, I
      would think that only a very few would give equal ids to two
      connections opened in sequence.  Would it not be better to check
      against all currently open connections?
    - You do not have a test that tests that brokered connections get
      the same id as the underlying physical connection.
    - Some of the lines are longer than 79 characters.

-- 
Øystein


Mime
View raw message