[IO-808] Rationalize and unify checking for existence of files and directories#529
Merged
Conversation
Member
|
@elharo |
asfgit
pushed a commit
that referenced
this pull request
Dec 21, 2023
Member
|
Hi @elharo PR merged, TY. I think we might now hear complaints that not all IO exceptions are documented. For example: |
Contributor
Author
|
It doesn't hurt to add more detailed exception documentation. If there are any public methods where FileNotFoundException is thrown and neither FileNotFoundException nor IOException is documented, then that should be fixed. FileNotFoundException is a subclass of IOException so it's not absolutely necessary to document it separately though it can be. |
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
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.
The existing code is a bit of a hodge-podge. This PR moves most current IllegalArgumentExceptions for non-existent files and directories to FileNotFoundExceptions instead. A few turned into UncheckedIOExceptions to maintain API compatibility. The general principle is:
IllegalArgumentExceptions are still thrown in the case where a file object exists but is a file instead of the expected directory or a directory instead of the expected file. I've come to understand that this is wrong too, but this is existing behavior that can be improved in a subsequent PR.
In addition to correctly reporting missing files, this PR removes a number of duplicate checks and methods, and tries to clarify the names of private methods, e.g. by distinguishing whether we're checking for a file, directory, or file object.
Some further changes that should happen here would break the API and require a major version update.
@garydgregory