Zipchem#424
Merged
Merged
Conversation
Cleanup for exception handling and explicit checking for null values.
* Moved to use Java 7 ZipFileSystem for appending files and refactored * ZipAppendUtil code to be within ZipChemCompProvider. * Cleaned up exception handling within ZipChemCompProvider * Created optional mode for ZipChemCompProvider that doesn't delete .cif.gz - this prevents redownloading .cif.gz with unit testing. * ZipChemCompProvider is thread-safe, with reads/appends in synchronized methods.
| private String zipRootDir = "chemcomp/"; | ||
| private ZipFile zipDictionary; | ||
| private String zipFile; | ||
| private static final Logger s_logger = Logger.getLogger( ZipChemCompProvider.class.getPackage().getName() ); |
Contributor
There was a problem hiding this comment.
This should be slf4j Logger, not the java.util.logging one.
Contributor
|
Looks good! If you can fix the logging and make it slf4j logging, we can then merge it in. Just in time for 4.2 :) |
…vider and unit test.
Contributor
Author
|
Oops! Good point - I've switched these two classes to use the slf4j logger On Mon, Mar 7, 2016 at 11:40 AM, Jose Manuel Duarte <
Matt Larson, PhD |
josemduarte
added a commit
that referenced
this pull request
Mar 7, 2016
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Jose suggested a clean up of ZipChemCompProvider for exception handling in general. The ZipFileSystem now available in Java 7 can manage the archiving and appending without backup copies of the archive, so I've switched to using this instead of ZipOutputStream and simplified the exception handling and error reporting. Unit testing with ZipChemCompProvider will no longer remove .cif.gz so as to avoid repeat downloading when running the tests involving ZipChemCompProvider and DownloadChemCompProvider. The class is threadsafe with synchronized methods for reading or appending to the archive.