> Am 02.02.2018 um 02:45 schrieb Stuart Marks <[email protected]>:
>
>
>
> 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
