commons-io
commons-io copied to clipboard
[IO-808] Report problems in the file system as IOExceptions
Problems that can't be determined from the argument alone, that is problems that aren't detected until the file system is read, are IOExceptions, not program bugs represented by IllegalArgumentException. No API changes. @garydgregory
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 86.12%. Comparing base (
858f5cf) to head (e0d6da0). Report is 927 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #586 +/- ##
============================================
+ Coverage 86.01% 86.12% +0.11%
+ Complexity 3499 3498 -1
============================================
Files 231 231
Lines 8250 8244 -6
Branches 960 960
============================================
+ Hits 7096 7100 +4
+ Misses 864 854 -10
Partials 290 290
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Quoting from the Javadoc of IllegalArgumentException: "Thrown to indicate that a method has been passed an illegal or inappropriate argument."
In other words, throwing an IAE is perfectly alright, there is no condition on how to detect the fact, that is the argument is illegal, or inappropriate.
I conclude, that this patch adds nothing to the existing code, at the cost of changing method signatures. I will not merge it.
Closing, I agree with @jochenw .
When the problem is detected a core difference between runtime and checked exceptions. Runtime exceptions indicate program bugs, like passing the wrong argument. Checked exceptions indicate environmental problems that are not known at compile time.
A file that does not exist is not an illegal or inappropriate argument. It's not the argument that's wrong. It's the file system. Consider this.
The file could exist when the argument is passed and then be deleted by a different process before the method tries to write or read it. The argument was 100% correct but there's still an exception. That shouldn't be an IllegalArgumentException. The argument wasn't the problem he I/O was.
Quoting @elharo: "A file that does not exist is not an illegal or inappropriate argument. "
That is, in my opinion, a matter of policy, hence questionable.
My personal opinion is this: It is the callers responsibility to ensure, that a file exists, before invoking an operation like copyFile, or moveFile. How that is ensured, is up to the caller. However, if you adopt that opinion, then it is clearly an illegal argument.
Whatever, I do not wish to continue this discussion. In the end, it is pointless. The current code works, as documented. (@throws IAE.) Even if we agreed on your policies, I'd still hold to the statement "Works as documented (perhaps not exactly as expected by you), so no need to fix anything."
The "policy" is set by the design of Java since 1.0. The current approach fights against how exceptions were designed to work in Java. Sure, you can do it that way and it compiles and perhaps is documented, but it's more than little like building a car where first gear is full highway cruising speed and fourth gear is for pulling a horse trailer up a mountain. It's going to confuse everyone who tries to drive it and probably cause a few accidents, and documenting this in the owners' manual really wouldn't help.