garydgregory commented on code in PR #391:
URL: https://github.com/apache/commons-io/pull/391#discussion_r990801985
##########
src/test/java/org/apache/commons/io/input/SequenceReaderTest.java:
##########
@@ -56,14 +60,86 @@ private void checkReadEof(final Reader reader) throws
IOException {
}
@Test
- public void testClose() throws IOException {
+ public void testAutoClose() throws IOException {
try (Reader reader = new SequenceReader(new
CharSequenceReader("FooBar"))) {
checkRead(reader, "Foo");
reader.close();
checkReadEof(reader);
}
}
+ @Test
+ public void testClose() throws IOException {
+ final Reader reader = new SequenceReader(new
CharSequenceReader("FooBar"));
+ checkRead(reader, "Foo");
+ reader.close();
+ checkReadEof(reader);
+ }
+
+ @Test
+ public void testCloseReaders() {
+ final AtomicBoolean closeEmptyReader = new AtomicBoolean(false);
+ final Reader emptyReader = new Reader() {
+ @Override
+ public int read(char[] cbuf, int off, int len) throws IOException {
+ if (closeEmptyReader.get()) {
+ throw new IOException("emptyReader already closed");
+ }
+
+ return EOF;
+ }
+
+ @Override
+ public void close() throws IOException {
+ closeEmptyReader.set(true);
+ }
+ };
+
+ final AtomicBoolean closeReader1 = new AtomicBoolean(false);
+ final Reader reader1 = new Reader() {
+ private final char[] content = new char[]{'A'};
+ private int position = 0;
+
+ @Override
+ public int read(char[] cbuf, int off, int len) throws IOException {
Review Comment:
Use `final` where you can.
##########
src/test/java/org/apache/commons/io/input/SequenceReaderTest.java:
##########
@@ -56,14 +60,86 @@ private void checkReadEof(final Reader reader) throws
IOException {
}
@Test
- public void testClose() throws IOException {
+ public void testAutoClose() throws IOException {
try (Reader reader = new SequenceReader(new
CharSequenceReader("FooBar"))) {
checkRead(reader, "Foo");
reader.close();
checkReadEof(reader);
}
}
+ @Test
+ public void testClose() throws IOException {
+ final Reader reader = new SequenceReader(new
CharSequenceReader("FooBar"));
+ checkRead(reader, "Foo");
+ reader.close();
+ checkReadEof(reader);
+ }
+
+ @Test
+ public void testCloseReaders() {
+ final AtomicBoolean closeEmptyReader = new AtomicBoolean(false);
+ final Reader emptyReader = new Reader() {
+ @Override
+ public int read(char[] cbuf, int off, int len) throws IOException {
+ if (closeEmptyReader.get()) {
+ throw new IOException("emptyReader already closed");
+ }
+
+ return EOF;
+ }
+
+ @Override
+ public void close() throws IOException {
+ closeEmptyReader.set(true);
+ }
+ };
+
+ final AtomicBoolean closeReader1 = new AtomicBoolean(false);
+ final Reader reader1 = new Reader() {
+ private final char[] content = new char[]{'A'};
Review Comment:
Use abbreviated notation, IOW `... = {'A'};`
##########
src/test/java/org/apache/commons/io/input/SequenceReaderTest.java:
##########
@@ -56,14 +60,86 @@ private void checkReadEof(final Reader reader) throws
IOException {
}
@Test
- public void testClose() throws IOException {
+ public void testAutoClose() throws IOException {
try (Reader reader = new SequenceReader(new
CharSequenceReader("FooBar"))) {
checkRead(reader, "Foo");
reader.close();
checkReadEof(reader);
}
}
+ @Test
+ public void testClose() throws IOException {
+ final Reader reader = new SequenceReader(new
CharSequenceReader("FooBar"));
+ checkRead(reader, "Foo");
+ reader.close();
+ checkReadEof(reader);
+ }
+
+ @Test
+ public void testCloseReaders() {
+ final AtomicBoolean closeEmptyReader = new AtomicBoolean(false);
+ final Reader emptyReader = new Reader() {
+ @Override
+ public int read(char[] cbuf, int off, int len) throws IOException {
Review Comment:
Use `final` where you can.
##########
src/main/java/org/apache/commons/io/input/SequenceReader.java:
##########
@@ -33,14 +36,16 @@ public class SequenceReader extends Reader {
private Reader reader;
private Iterator<? extends Reader> readers;
+ private Iterable<? extends Reader> readersIterable;
Review Comment:
No need to change the name, it makes the PR noisier.
##########
src/test/java/org/apache/commons/io/input/SequenceReaderTest.java:
##########
@@ -56,14 +60,86 @@ private void checkReadEof(final Reader reader) throws
IOException {
}
@Test
- public void testClose() throws IOException {
+ public void testAutoClose() throws IOException {
try (Reader reader = new SequenceReader(new
CharSequenceReader("FooBar"))) {
checkRead(reader, "Foo");
reader.close();
checkReadEof(reader);
}
}
+ @Test
+ public void testClose() throws IOException {
+ final Reader reader = new SequenceReader(new
CharSequenceReader("FooBar"));
+ checkRead(reader, "Foo");
+ reader.close();
+ checkReadEof(reader);
+ }
+
+ @Test
+ public void testCloseReaders() {
+ final AtomicBoolean closeEmptyReader = new AtomicBoolean(false);
+ final Reader emptyReader = new Reader() {
+ @Override
+ public int read(char[] cbuf, int off, int len) throws IOException {
+ if (closeEmptyReader.get()) {
+ throw new IOException("emptyReader already closed");
+ }
+
+ return EOF;
+ }
+
+ @Override
+ public void close() throws IOException {
+ closeEmptyReader.set(true);
+ }
+ };
+
+ final AtomicBoolean closeReader1 = new AtomicBoolean(false);
+ final Reader reader1 = new Reader() {
+ private final char[] content = new char[]{'A'};
+ private int position = 0;
+
+ @Override
+ public int read(char[] cbuf, int off, int len) throws IOException {
+ if (closeReader1.get()) {
+ throw new IOException("reader1 already closed");
+ }
+
+ if (off < 0) {
+ throw new IndexOutOfBoundsException("off is negative");
+ } else if (len < 0) {
+ throw new IndexOutOfBoundsException("len is negative");
+ } else if (len > cbuf.length - off) {
+ throw new IndexOutOfBoundsException("len is greater than
cbuf.length - off");
+ }
+
+ if (position > 0) {
+ return EOF;
+ }
+
+ cbuf[off] = content[0];
+ position++;
+ return 1;
+ }
+
+ @Override
+ public void close() throws IOException {
+ closeReader1.set(true);
+ }
+ };
+
+ try (SequenceReader sequenceReader = new SequenceReader(reader1,
emptyReader)) {
+ assertEquals('A', sequenceReader.read());
+ assertEquals(EOF, sequenceReader.read());
+ } catch (IOException e) {
+ fail("No IOException expected");
Review Comment:
Simpler is better:
```
try (SequenceReader sequenceReader = new SequenceReader(reader1,
emptyReader)) {
assertEquals('A', sequenceReader.read());
assertEquals(EOF, sequenceReader.read());
} finally {
assertTrue(closeEmptyReader.get());
assertTrue(closeReader1.get());
}
```
##########
src/test/java/org/apache/commons/io/input/SequenceReaderTest.java:
##########
@@ -56,14 +60,86 @@ private void checkReadEof(final Reader reader) throws
IOException {
}
@Test
- public void testClose() throws IOException {
+ public void testAutoClose() throws IOException {
try (Reader reader = new SequenceReader(new
CharSequenceReader("FooBar"))) {
checkRead(reader, "Foo");
reader.close();
checkReadEof(reader);
}
}
+ @Test
+ public void testClose() throws IOException {
+ final Reader reader = new SequenceReader(new
CharSequenceReader("FooBar"));
+ checkRead(reader, "Foo");
+ reader.close();
+ checkReadEof(reader);
+ }
+
+ @Test
+ public void testCloseReaders() {
+ final AtomicBoolean closeEmptyReader = new AtomicBoolean(false);
+ final Reader emptyReader = new Reader() {
+ @Override
+ public int read(char[] cbuf, int off, int len) throws IOException {
+ if (closeEmptyReader.get()) {
+ throw new IOException("emptyReader already closed");
+ }
+
+ return EOF;
+ }
+
+ @Override
+ public void close() throws IOException {
+ closeEmptyReader.set(true);
+ }
+ };
+
+ final AtomicBoolean closeReader1 = new AtomicBoolean(false);
Review Comment:
No need to initialize an `AtomicBoolean` to `false`, that's the default, IOW
-> `new AtomicBoolean();`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
