kvr000 commented on PR #455: URL: https://github.com/apache/commons-compress/pull/455#issuecomment-1890754622
> Hello @kvr000 Thank you for your patience. Instead of adding yet another constructor, I've refactored the code to use a `ZipFile` builder (see `ZipFile.Builderr`). I think this is better than an options object builder. You can add and use the options you need if any for this PR to `ZipFile.Builder`. The options builder should no longer be needed so you can ignore the comments I made there. TY! Hi Gary, thanks for your response. The `ZipFile` builder certainly works too, in a way, it's a shortcut to pass parameters directly instead of via another object. The reason why I chose separate Options object is: - It's pretty common pattern for complex initialization. It is actually used in SevenZFileOptions as well. - The configuration is typically made once and then reused through the application. Having said that, to make it fully reusable, I should have probably passed the path separately, though `toBuilder().modify().build()` still works. - It should be more reusable across different archivers, both readers and writers. Some would support only subset but generally, having single ZipOptions would allow sharing at least among ZipFile, ZipArchiveInputStream and ZipArchiveOutputStream. I'm fine with the current option as well though. Please let me know whether the current choice is final and if so, I can adjust my code. I'd still recommend passing full `Builder` to constructor instead of all arguments separately - it scales better and is less prone to incompatible changes. -- 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]
