RFR: 8057113: (fs) Path should have a method to obtain the filename extension [v4]

[Brian Burkhalter]

On Wed, 27 Apr 2022 18:12:35 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8057113: Implement reviewer suggestions for replaceExtension()
>
> src/java.base/share/classes/java/nio/file/Path.java line 262:
> 
>> 260:      * the path has zero elements ({@link #getFileName()} returns
>> 261:      * {@code null}), or the file name string does not contain a dot, only
>> 262:      * the first character is a dot, or the last character is a dot.
> 
> Musing on a filename with a final dot, i.e.  `FooBar.`
> I think it has an empty string "" as the extension (because there is a dot).
> 
> As described, if the `defaultExtension` is returned, then it would be nice for the code to trust
> that there is no final dot.  If it is case when the `defaultExtension` is returned, there may or may not be a final dot then the caller may need to test separately for it before using the filename.

Currently a filename with a terminating dot is considered not to have an extension. Yes, `defaultExtension` should have been checked for a dot.

> src/java.base/share/classes/java/nio/file/Path.java line 278:
> 
>> 276:      *
>> 277:      * @return  the file name extension of this path, or
>> 278:      *          {@code defaultExtension} if the extension is indeterminate
> 
> Seeing the use of `Optional<String> below`, returning an `Optional<String>` for this method might also be a good idea. (And dropping the `defaultExtension` parameter).
> The testing for a missing extension is more obvious and the substitution for a missing extension can be handled by `Optional.orElse(`).

I agree that returning an `Optional<String>` is a good idea. Getting rid of the parameter is a good thing. Commit 8ed8b1b25efa18a419d9176bff38f12616c9881d does this.

> src/java.base/share/classes/java/nio/file/Path.java line 356:
> 
>> 354:      *         but is not otherwise checked for validity
>> 355:      *
>> 356:      * @return a new path equal to this path but with extension replaced by
> 
> Drop the word "new"; so it can return the existing Path if there is no change.

Commit 8ed8b1b25efa18a419d9176bff38f12616c9881d fixes this.

> src/java.base/share/classes/java/nio/file/Path.java line 370:
> 
>> 368:         if (!extension.isEmpty()) {
>> 369:             if (extension.indexOf('.') == 0 ||
>> 370:                 extension.lastIndexOf('.') == extension.length() - 1)
> 
> No need to scan the string using `indexOf`:
> 
> `(extension.charAt(0) == '.' || extension.charAt(extension.length()-1) == '.')`

Commit 8ed8b1b25efa18a419d9176bff38f12616c9881d fixes this.

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

PR: https://git.openjdk.java.net/jdk/pull/8066