Thanks Bryan for reading my patch. Please see my answers below: On 2/26/06, Bryan Pendleton <[EMAIL PROTECTED]> wrote:
> Three small comments: > > 1) It wasn't clear to me why these needed to be applied as two > separate patches. I would be comfortable putting the test and the > fix in the same commit; is there something I'm missing here? The patches can be committed together. It is just that patch2 (tests) should not be committed before patch1 (code changes). I'll mention this when I upload the next version of patch. > > 2) There's a mysterious comment in DRDAStatement.initialize(): > > + * TODO: Need to see what exactly it means to initialize the default > + * statement. (DERBY-1002) > > Does this mean that there is still a loose end to tie up here? > Yes. The comments in the code where DRDAStatement.initialize() is being called is confusing. They say "// initialize statement for reuse". I think this is confusing because we have reset() method which also is called to re-use statements stored in stmtTable. initialize() is called for default statement, which is used differently. From what I understand, there is just one default statement per database and default statement is different in the way it gets initialized and used. e.g: stmt variable used in default statement is created only once in Database.makeConnection. So, we cannot call reset() for default statement. I think it would be good to understand this part and fix up the comments/code so that it is clear. Kathey and Knut also mentioned that the use of initialize method at few places was confusing to them. Sorry, I did not mention explicitly that the patch has more to-do items in my latest comment. > 3) It took me a little while to realize that the idea of testStatementReuse > is that it sets up the section tables in such a way as to provoke statement > re-use in tests jira_491_492 and testImplicitClose. [snip ... very good suggestions to improve comments in the test case] Thanks for the suggestions. I'll fix the comments and upload a new version of patch2. Thanks, Deepa
