kvr000 commented on PR #455: URL: https://github.com/apache/commons-compress/pull/455#issuecomment-1872616449
> * Big picture: I am considering adding some random IO classes to Apache Commons IO like Apache PDFBox has, see https://github.com/apache/pdfbox/tree/trunk/io/src/main/java/org/apache/pdfbox/io That totally makes sense, each library provides its own IOUtils currently. I'd suggest few things: 1. FileChannel is typically more meaningful than SeekableByteChannel. Even here in ZipFile and related, the SeekableByteChannel is used in a sense of FileChannel. I provided fix for this a while ago which allowed lock-free reading from multiple ZipFile entries in parallel but there is the same work in other parts. 2. The JDK unfortunately misses interface for position_read and position_write methods which makes custom implementations more tricky. I was addressing this issue in https://github.com/dryuf/dryuf-bigio (FlatChannel), you may consider this for inspiration when working on commons-io stuff. 3. The PdfBox in my opinion is quite complicated and may suffer from diamond inheritance problem. The above may be more straightforward and more efficient. > * In light of this, it is good that the random IO classes you've added are package-protected. This will let us possibly replace this implementation with this very code ported to Commons IO or with code from PDFBox ported to Commons IO. We do not want Commons Compress to depend on PDFBox, especially since PDFBox already depends on Commons IO. > * To reduce the new public API surface, please move the new IOUtils method into package-protected place in the `zip` package. That was done. > * I'll study all of this some more and see what is generic enough to add and reuse from Commons IO. It may require more work to make it more generic. For now the package private is probably better choice. -- 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]
