garydgregory commented on PR #784: URL: https://github.com/apache/commons-io/pull/784#issuecomment-3318849154
All good topics, some comments below. > * It mixes sources and sinks. We could clean this up by splitting into two interfaces (e.g. `InputOrigin` and `OutputOrigin`), or by adding `isReadable()` / `isWritable()` markers. Hm, that would only work for a subset of origins. Would you leave origins like `Path`, `File`, and `SeekableByteChannel` under `AbstractOrigin`, or, create an `AbstractIoOrigin`? (I don't think we'd want to test a given file's attributes since these can change on the fly.) I can see that you could move `InputStreamOrigin` and `ReaderOrigin` under an `AbstractInputOrigin`. Would this really buy us anything? > Resource ownership semantics are unclear. Some origins wrap pre-opened resources (InputStream, RandomAccessFile, Channel), others create new resources on demand (byte[], File, Path, URI). Right now it’s ambiguous who “owns” the underlying resource. In practice, the builder should not take ownership unless explicitly documented; otherwise returned resources should be close-shielded. I thought this was all clear to: The allocator owns the resources and must close it. Are there any bugs? > wrap pre-opened resources (InputStream, RandomAccessFile, Channel), This falls under "the allocating call site owns the resource." > others create new resources on demand (byte[], File, Path, URI). There is nothing to close on these types, they are not "resources" in the traditional "implements AutoCloseable" sense. What am I missing? > The semantics of getByteArray() are also fuzzy: it consumes the origin, but should that also imply closing it? This falls under "the allocating call site owns the resource". This should be covered by existing unit tests. Do you see some missing or incorrect cases? All good topics 😄 -- 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]
