ppkarwasz commented on code in PR #784:
URL: https://github.com/apache/commons-io/pull/784#discussion_r2366902443


##########
src/main/java/org/apache/commons/io/build/AbstractOriginSupplier.java:
##########
@@ -368,4 +376,26 @@ public B setURI(final URI origin) {
     public B setWriter(final Writer origin) {
         return setOrigin(newWriterOrigin(origin));
     }
+
+    /**
+     * Sets a new origin.
+     *
+     * @param origin the new origin.
+     * @return {@code this} instance.
+     * @since 2.21.0
+     */
+    public B setReadableByteChannel(final ReadableByteChannel origin) {
+        return setOrigin(newChannelOrigin(origin));
+    }
+
+    /**
+     * Sets a new origin.
+     *
+     * @param origin the new origin.
+     * @return {@code this} instance.
+     * @since 2.21.0
+     */
+    public B setWritableByteChannel(final WritableByteChannel origin) {

Review Comment:
   I considered a single `setChannel(Channel)` as well, but decided on separate 
`setReadableByteChannel` and `setWritableByteChannel` methods for type safety.
   
   While any `Channel` can be wrapped in a `ChannelOrigin`, the actual use 
cases differ: you need a `ReadableByteChannel` to expose an `InputStream`, and 
a `WritableByteChannel` to expose an `OutputStream`. Keeping the methods 
separate lets the compiler catch mismatches instead of deferring to runtime.
   
   On the other hand, many channel implementations declare both interfaces but 
only fully support one. In those cases, errors surface **only** at runtime. Do 
you think it would be clearer to collapse this into a single `setChannel` 
method instead?
   



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

Reply via email to