Hello.
I have uploaded DERBY-326_3.patch.
Next are response for each comments of Bryan.
Bryan Pendleton wrote:
1) It was hard for me to understand the purpose of the class
ReEncodedInputStream.java simply from reading the code. I
think it would be good for you to add some comments describing
the purpose of this class and its expected uses.
I have added documentation comment.
Please confirm it.
2) It seems like it might be possible to improve these lines in
ReEncodedInputStream.java:
if(reader == null){
throw new NullPointerException();
}
if(enc == null){
throw new NullPointerException();
}
I think it would be better to pass a string to the exception
constructor, indicating which variable was null, as in:
throw new NullPointerException(
"null 'reader' passed to ReEncodedInputStream");
That way, if there are no line numbers available in the stack
trace, we'll still be able to tell the two exceptions apart.
I have added those message to exception.
3) In EXTDTAInputStream.getEXTDTAStream(), I did not understand
the purpose of the "is" InputStream variable. It seems to be
initialized to null, then checked in the finally{} block, but
is otherwise unused. Am I missing something? Or can this
variable be removed?
Thank you.
They were unused variables written in the middle of coding and testing.
I removed them.
4) In EXTDTAInputStream.openInputStreamLazily, I think it would be
better to change the line
throw new IllegalStateException();
to
throw new IllegalStateException(
"Either 'blob' or 'clob' must be non-null");
I have added those information to the exception.
5) Also in EXTDTAInputStream.openInputStreamLazily, what is supposed
to happen in the empty catch block for UnsupportedEncodingException?
Do you really intend just to ignore this exception? If so, then
I think it would be good to add some comments explaining why it
is appropriate to catch and ignore this exception.
I have added comment to catch block of UnsupportedEncodingException.
6) Are the comments above DDMWriter.writeScalarStream() still
accurate? It looks like you have addressed some of these issues,
but you didn't seem to change the comment. It would be good to
check the comment and see if it needs to be updated.
Thank you ...
I read the comment and take some important information.
Issues of ToDo Comment here was finished at server side.
However, comment says that there exists similar problem at client.
I have not touched client side code yet and want to leave it for another
patch.
I moved the comment to client side.
7) It seems like the writeScalarStream method and its related methods
in DDMWriter.java (prepScalarStream, flushScalarStreamSegment, etc.)
do not use the "ensureLength()" protocol which is common elsewhere
in DDMWriter.java, but instead they examine the buffer length by
checking bytes.length and then being careful not to overrun that
size. I'm not sure exactly why these methods are different from all
the other data-writing methods in this respect, but I think it would
be nice to add some comments to the code describing this behavior.
The ensureLength() method was not used in original.
I don't know why.
However, reading the code around, I found that size of buffer can be
very small depending on parameters for constructor and,
call to ensureLength will adjust the size of buffer appropriate.
I added call to ensureLength() method in writeScalarStream().
Please read and review the patch again.
Best regards.
--
/*
Tomohito Nakayama
[EMAIL PROTECTED]
[EMAIL PROTECTED]
[EMAIL PROTECTED]
Naka
http://www5.ocn.ne.jp/~tomohito/TopPage.html
*/
