kinow commented on a change in pull request #192:
URL: https://github.com/apache/commons-io/pull/192#discussion_r565660147
##########
File path: src/main/java/org/apache/commons/io/FileUtils.java
##########
@@ -1168,7 +1169,9 @@ static String decodeUrl(final String url) {
*/
public static File delete(final File file) throws IOException {
Objects.requireNonNull(file, "file");
- Files.delete(file.toPath());
+ Path path = file.toPath();
+ Files.delete(path);
+ assert Files.notExists(path, PathUtils.NOFOLLOW_LINK_OPTION_ARRAY) :
"File still exists " + path;
Review comment:
I'm not sure if that's really necessary to have in this method? From the
docs for this file, it used to be a useful alternative for `File#delete` that
returned a boolean. But now with the JVM (from 8 I think?) `Files#delete` they
both have similar behaviour.
So if the file cannot be deleted, an exception is thrown. If the JVM
implementation (or underlying OS libraries, or maybe a NFS or virtual file
system) reported the file as deleted, but the file still exists, then both
`Files#delete` and this method will behave the same way.
I'm not sure if having an assertion error here would be helpful to users of
these platforms. They'd probably have to rely on the JVM, or have their code to
check for the file still existing (they could be using the JVM `Files#delete`
somewhere else in the code).
##########
File path: pom.xml
##########
@@ -394,6 +394,7 @@ file comparators, endian transformation classes, and much
more.
</classpathDependencyExcludes>
<forkCount>1</forkCount>
<reuseForks>false</reuseForks>
+ <enableAssertions>true</enableAssertions>
Review comment:
I haven't used or read about assertions in Java in a while. Unless
something changed in recent releases, I think this enables the `-ea` flag for
build time. But users of the API would still have it disabled by default in
OpenJDK/Oracle JVM's.
So the new code with assertions wouldn't be used, and I think using
assertions in production code (not in tests) used to be discouraged?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
