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

[Brian Burkhalter]

On Fri, 14 Jul 2023 11:11:22 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> 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.

Addressed in f78894c86eeeb4c94ed417eae43793c9dd1c80ee.

> 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.

Addressed in f78894c86eeeb4c94ed417eae43793c9dd1c80ee.

> 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" ?

Addressed in f78894c86eeeb4c94ed417eae43793c9dd1c80ee.

> 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".

Addressed in f78894c86eeeb4c94ed417eae43793c9dd1c80ee.

> 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".

Addressed in f78894c86eeeb4c94ed417eae43793c9dd1c80ee.

> 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".

Addressed in f78894c86eeeb4c94ed417eae43793c9dd1c80ee.

> 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.

Addressed in f78894c86eeeb4c94ed417eae43793c9dd1c80ee.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263933203
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263933271
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263933418
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263933651
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263933816
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263933733
PR Review Comment: https://git.openjdk.org/jdk/pull/14805#discussion_r1263933541