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

[Brian Burkhalter]

On Thu, 21 Apr 2022 21:15:40 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: Add hasExtension() and replaceExtension()
>
> src/java.base/share/classes/java/nio/file/Path.java line 326:
> 
>> 324:             return Optional.empty();
>> 325: 
>> 326:         Objects.requireNonNull(ext);
> 
> Typically, all of the argument checks are done first. 
> Though it does create a strange shape for the code when there is no extension. Two passes over the extensions seems like a waste.

Yes, I thought about this. It did not seem to be worthwhile to pass through the args just to check for a null parameter that one might not even need to use.

> src/java.base/share/classes/java/nio/file/Path.java line 372:
> 
>> 370:         int dotIndex = extension.lastIndexOf('.');
>> 371:         if (dotIndex > 0)
>> 372:             throw new IllegalArgumentException();
> 
> Can this be stricter and not include a conditional "."
> It would be more consistent to define the extension as never including the ".".

That would be fine with me and would simplify the code.

> src/java.base/share/classes/java/nio/file/Path.java line 384:
> 
>> 382:         if (thisExtension == null)
>> 383:             return extension.isEmpty() ?
>> 384:                 this : Path.of(path + "." + extension);
> 
> Path.of would revert to the default file system, even if the current Path is from a different FS.
> Can the new path be defined from the current FS?

That would likely be better: `getFilesystem().getPath(String)`.

> src/java.base/share/classes/java/nio/file/Path.java line 395:
> 
>> 393:         StringBuilder sb = new StringBuilder(path.substring(0, dotIndex + 1));
>> 394:         sb.append(extension);
>> 395:         return Path.of(sb.toString());
> 
> Is using StringBuilder more efficient in space than just using String + string concat?

For just two elements like here it is debatable.

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

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