On Fri, 29 Nov 2024 13:33:37 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
> Please review this PR which applies various cleanups to
> `sun.net.www.protocol.file.FileURLConnection`.
>
> This class is known to be an _old, intricate, and hard to maintain piece of
> code._ However, there are some relatively straightforward refactorings /
> cleanups possible which improve readability and makes it easier to reason
> about what's going on in this class.
>
> In this PR, I have chosen to make each individual small change a separate
> commit. This to assist review of each individual change, which can otherwise
> disappear a bit when reviewing the PR as a whole.
>
> A detailed listing of each commit follows in a separate comment.
>
> This is a pure cleanup PR, no tests are added or updated in this PR.
src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java
line 78:
> 76: if (fileList == null)
> 77: throw new FileNotFoundException(file.getPath() + "
> exists, but is not accessible");
> 78: directoryListing = Arrays.<String>asList(fileList);
Is the `<String>` parameterization necessary? I tested on jshell, and it seems
since the destination variable's type is `List<String>` we don't need this
explicit parameterization to distinguish varargs.
src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java
line 193:
> 191: String fileName = directoryListing.get(i);
> 192: sb.append(fileName);
> 193: sb.append("\n");
We can use a `StringJoiner` or stream to join the strings instead. (Note there
is a suffix `\n` in the strings)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22459#discussion_r1863735620
PR Review Comment: https://git.openjdk.org/jdk/pull/22459#discussion_r1863738691
