maxxedev commented on code in PR #748:
URL: https://github.com/apache/commons-io/pull/748#discussion_r2094193320


##########
src/main/java/org/apache/commons/io/input/QueueInputStream.java:
##########
@@ -224,4 +226,52 @@ public int read() {
         }
     }
 
+    /**
+     * Reads up to {@code length} bytes of data from the input stream into
+     * an array of bytes.  The first byte is read while honoring the timeout; 
the rest are read while <i>not</i> honoring
+     * the timeout. The number of bytes actually read is returned as an 
integer.
+     *
+     * @param b     the buffer into which the data is read.
+     * @param offset   the start offset in array {@code b} at which the data 
is written.
+     * @param length   the maximum number of bytes to read.
+     * @return     the total number of bytes read into the buffer, or {@code 
-1} if there is no more data because the
+     *              end of the stream has been reached.
+     * @throws IllegalStateException if thread is interrupted while waiting 
for the first byte.
+     * @throws IndexOutOfBoundsException if {@code offset} is negative, {@code 
length} is negative, or {@code length} is
+     *             greater than {@code b.length - offset}.
+     */
+    @Override
+    public int read(byte[] b, int offset, int length) {
+        if (offset > b.length || offset < 0) {
+            throw new IndexOutOfBoundsException("Offset out of bounds: " + 
offset);
+        }
+        if (length < 0 || length > b.length - offset) {
+            throw new IndexOutOfBoundsException("Length out of bounds: " + 
length);
+        }
+
+        int i = 0;
+        if (length > 0) {
+            // read the first value, honoring the timeout
+            final int firstValue = read();
+            if (firstValue == EOF) {
+                return EOF;
+            }
+            b[offset + i] = (byte) (0xFF & firstValue);
+            i++;
+        }
+
+        if (length > 1) {
+            // read the rest, ignoring the timeout
+            final List<Integer> drain = new ArrayList<>(length * 2);
+            blockingQueue.drainTo(drain, length - 1);

Review Comment:
   > Why is the initial capacity of the list twice the number of expected 
elements?
   
   Ah, I got confused with HashMap loadFactor. ArrayList does not need such 
buffer
   &nbsp;
   
   > Would it make sense to store the list in a field to avoid recreating it on 
each call? Unless I'm missing something, QueueInputStream doesn't appear to 
support concurrent reads, so we might not need to worry about thread safety in 
this case.
   
   I would prefer to keep it simple and also maintain strict thread-safety. The 
javadoc says: "queue input/output streams may be used safely in a single thread 
or multiple threads". In practice, that means QueueInputStream and 
QueueOutputStream _together_ can be used concurrently (i.e. one thread reading 
and one thread writing). There is no practical reason someone would want to 
read from QueueInputStream concurrently (i.e. two threads reading). Regardless 
I want to avoid having to document such theoretical thread un-safety and avoid 
confusing the users.



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to