RFR 8196298 Add null Reader and Writer

Patrick Reinhart patrick at reini.net
Sat Feb 3 17:05:36 UTC 2018



> Am 02.02.2018 um 02:45 schrieb Stuart Marks <stuart.marks at oracle.com>:
> 
> 
> 
> 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.
> 

I stated in the nullWriter() method that flush() will have no action. I certainly can implement that behavior, so in consequence I will change the API description of the nullWriter() method accordingly.


> **
> 
> 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
> 

I reworked the tests and Writer implementation accordingly

http://cr.openjdk.java.net/~reinhapa/reviews/8196298/webrev.01

-Patrick




More information about the core-libs-dev mailing list