RFR: 8262742: (fs) Add Path::resolve with varargs string [v4]

[Alan Bateman]

On Wed, 12 Jul 2023 17:09:25 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Add to `java.nio.file.Path` methods which allow resolution of multiple descendants.
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8262742: Attempt to improve specifications of varargs resolve methods

src/java.base/share/classes/java/nio/file/Path.java line 521:

> 519:     /**
> 520:      * Resolves one or more {@code Path}s iteratively against this
> 521:      * {@code Path}. If {@code more} does not specify any {@code Path}s,

The. updated text looks much better but I think we need to make it clearer that it's only "first" that is resolved against this path. How about this for the first sentence: "Resolves a path against this path, and then iteratively resolves any additional paths". This is deliberately brief to avoid a complicated first sentence.

src/java.base/share/classes/java/nio/file/Path.java line 552:

> 550:      * @param more additional paths to be iteratively resolved against this path
> 551:      *
> 552:      * @return the resulting path

Minor comment is that @param/@return style is a bit different to the other methods.

src/java.base/share/classes/java/nio/file/Path.java line 568:

> 566:     /**
> 567:      * Resolves one or more {@code Path}s converted from the supplied path
> 568:      * srings iteratively against this {@code Path}. If {@code more} does not

How about "Converts a  path string to a Path, resolves it against this path, and then iteratively converts and resolves any additional path strings" ?

src/java.base/unix/classes/sun/nio/fs/UnixPath.java line 398:

> 396:     }
> 397: 
> 398:    private static final byte[] resolve(byte[] base, byte[]... descendants) {

When doing resolution, the term used is often "child". The existing 2-arg resolve uses "child" for example. So I think this should be renamed to "children".

src/java.base/unix/classes/sun/nio/fs/UnixPath.java line 402:

> 400:        // descendant is that last one which is an absolute path
> 401:        int start = 0;
> 402:        int length = base.length;

One suggestion here is to rename this to "resultLength".

src/java.base/unix/classes/sun/nio/fs/UnixPath.java line 409:

> 407:        if (count > 0) {
> 408:            for (int i = 0; i < count; i++) {
> 409:                byte[] b = descendants[i];

This can be renamed to "child".

src/java.base/unix/classes/sun/nio/fs/UnixPath.java line 428:

> 426:        // account for an extra '/' added in the length computation.
> 427:        if (start == 0 && length > base.length &&
> 428:            base.length == 1 && base[0] == '/')

The line break make is very hard to read.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263610126
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263611298
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263614505
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263619666
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263693991
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263692334
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263615119