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