2010/6/15 Ivan Maidanski <iv...@mail.ru>: > Hi! > > Here, the proposed changes of PrintStream are: > 1. to be closer to the RI behavior (see changes for line_separator, > out.flush(), append());
While some of these are sensible changes, how are you discovering the 'RI behaviour' in the cases where it wouldn't be visible from simple runtime testing? > 2. some code optimizations (like removal of writeChars() pos/len parameters). > > ChangeLog entries: > * java/io/PrintStream.java (line_separator): Convert static field to > an instance one (to match the RI functionality). I don't see why you would want to make this an instance field. > * java/io/PrintStream.java (error_occurred): Remove an unnecessary > initialization to false. I think having it explicit does make things clearer. > * java/io/PrintStream.java (PrintStream): Use > "new FileOutputStream(fileName)" instead of > "new FileOutputStream(new File(fileName))". Ok (there are two instances of this, your ChangeLog only lists one) > * java/io/PrintStream.java (PrintStream): Throw NPE if out is null. Where is this specified? The Javadoc for this don't specify an NPE is thrown. If OpenJDK does, then the documentation needs updating. We should include a message saying what the problem was. > * java/io/PrintStream.java (writeChars, print, println): Don't pass > "pos" and "len" parameters to writeChars() (as they are always set > to 0 and buf.length, respectively). I think the original version should stay. It's clearer and avoids a method call. This change occurs twice but is listed here only once. > * java/io/PrintStream.java (print): Call out.flush() only if needed > (to match the RI). Where are these extra conditions coming from? > * java/io/PrintStream.java (lastIndexOfNewLine): New private static > method (called from print() only). > * java/io/PrintStream.java (write): Remove unnecessary "&" operation. > * java/io/PrintStream.java (print, write): Directly call out.flush() > instead of flush() (the same as in the RI). > * java/io/PrintStream.java (append): Call subSequence() also for > "null" string (the same as in the RI). > > The append change looks good as do the flush() changes. I don't see the reason for changing the ordering of the && statement as checking auto_flush is the cheaper check. Additional arguments should stay. The ChangeLog needs some work as it was hard to follow and doesn't match up to the patch. -- Andrew :-) Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) Support Free Java! Contribute to GNU Classpath and the OpenJDK http://www.gnu.org/software/classpath http://openjdk.java.net PGP Key: 94EFD9D8 (http://subkeys.pgp.net) Fingerprint: F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
