RFR 8196298 Add null Reader and Writer
Stuart Marks
stuart.marks at oracle.com
Fri Feb 2 01:45:12 UTC 2018
On 2/1/18 10:35 AM, Patrick Reinhart wrote:
> Here is my first shot of the actual implementation and tests:
>
> http://cr.openjdk.java.net/~reinhapa/reviews/8196298/webrev.00
I looked at the specification of ready() vs the implementation. The spec says
that ready(), among other methods, behaves as if the Reader is at EOF. The spec
for Reader.ready() says "true if the next read() is guaranteed not to block for
input." Thus I'd assume that at EOF, read() will return -1 immediately, without
blocking, so ready() should return true.
The NullReader implementation inherits ready() from Reader, which always returns
false.
So, the behavior doesn't seem consistent with my reading of the specs. However,
other implementations aren't, either. For example, the spec for
CharArrayReader.ready() says "Character-array readers are always ready to be
read." However,
new CharArrayReader(new char[0]).ready()
returns false!
A couple other readers I tried (InputStreamReader, FileReader) also have ready()
return true until they reach EOF, at which time they return false. But
StringReader.ready() always returns true. Hmmmm.
Maybe the safest thing to do is to leave the various Readers' behavior of
ready() as they are, and leave nullReader's behavior to be consistent with them.
No changes are necessary to Reader in this webrev.
We should probably take a pass over the ready() methods in the Reader family to
see if they're correct. I'd suggest they need adjustment to mean that the next
read() won't block *and* data is available. And the CharArrayReader.ready() spec
should be fixed.
**
On the Writer side, the spec should mention that the three append() methods and
five write() methods do nothing if open and throw IOE if the Writer is closed.
The Writer.flush() method spec doesn't say what happens if flush() is called
after the Writer is closed. However, Writer.close() says that subsequent write()
or flush() calls throw IOE. Writer implementations that track open/closed state,
such as FileWriter, will throw IOE from flush() after closure. Since it's also
tracking open/closed state, I'd expect nullWriter.flush() to do the same. That
is, do nothing if open, throw IOE if closed.
Tests for flush() should be added.
**
Speaking of tests, the tests for proper exception-throwing for the closed cases
can probably usefully employ the
@Test(expectedExceptions=IOException.class)
annotation. The reason I often avoid this construct is that if the same
exception class is thrown from an unexpected part of the test, it can mask
actual failures. However, since these are one-liners, that reason doesn't apply.
Thus:
@Test(groups="closed", expectedExceptions=IOException.class)
public static void testAppendCharClosed() {
closedWriter.append('x');
}
You can probably also omit the try/catch block from the open tests, so that
instead of
@Test(groups="open")
public static void testAppendChar() {
try {
assertSame(openWriter, openWriter.append('x'));
} catch (IOException e) {
fail("Unexpected IOException");
}
}
you can write
@Test(groups="open")
public static void testAppendChar() {
assertSame(openWriter, openWriter.append('x'));
}
since the framework should catch and report errant exceptions, without the need
for code to catch exceptions and call fail().
s'marks
More information about the core-libs-dev
mailing list