ppkarwasz commented on PR #784: URL: https://github.com/apache/commons-io/pull/784#issuecomment-3317758478
Hi @garydgregory, I originally had a `getSeekableByteChannel()` in this PR but removed it because it felt misleading. For example, you can construct a `SeekableByteChannel` from a `FileOutputStream`, but that is obviously *not* the kind of channel Compress expects. My current approach is to call `getReadableByteChannel()` in Compress and, if necessary, check `instanceof SeekableByteChannel`. This works in practice: see the test that covers 9 different ways of configuring a `SeekableByteChannel`: https://github.com/apache/commons-io/blob/0c853487783a43abb85de3fcc204ecca3a5ca421/src/test/java/org/apache/commons/io/build/AbstractStreamBuilderTest.java#L119-L133 BTW, I think that `AbstractOrigin` is a bit muddled right now: * 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. * 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. * In Compress, builders are typically used to create an `ArchiveInputStream`, which *does* close the underlying resource when closed. That behavior should probably be made explicit in the Javadocs for `getInputStream()` and `getReadableByteChannel()`. * The semantics of `getByteArray()` are also fuzzy: it consumes the origin, but should that also imply closing it? -- 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]
