<Swing Dev> [9] Review Request for 8081722: Provide public API for file hierarchy provided by sun.awt.shell.ShellFolder

[Semyon Sadetsky]

Please look at the updated webrev: 
http://cr.openjdk.java.net/~ssadetsky/8081722/webrev.05/
Changes was made According to off-line discussion wit Sergey:

- IAE -> NPE for file == null
- FNFE is corrected for getLinkLocation()

--Semyon

On 1/22/2016 9:34 AM, Semyon Sadetsky wrote:
>
>
> On 1/21/2016 6:25 PM, Sergey Bylokhov wrote:
>> On 21/01/16 09:16, Semyon Sadetsky wrote:
>>>>
>>>> There are still inconsistency. The isFileSystem(), isParent(),
>>>> getSystemIcon, getSystemDisplayName() etc do not throw an exception in
>>>> case of null, but return some default value(null,true,false). Same for
>>>> FileNotFoundException() most of the methods just catch this exception
>>>> and return the default value.
>>> The methods you've mentioned do not throw NPE but other methods in the
>>> FSV class do throw NPE if called with null argument.
>>
>> they raise NPE not an IAE. ok I am fine to use NPE.
>>
>> Some other issue is inconsistency between isLink() vs getLinkLocation()
>> getLinkLocation() should "Returns {@code null} if the specified file 
>> is not a shell interpreted link", but isLink() return false if 
>> FileNotFoundException which means that getLinkLocation() should 
>> return null;
>>
>> the next code can work incorrectly:
>>         FileSystemView fsv = FileSystemView.getFileSystemView();
>>         File file = "some unexisted file";
>>         if (fsv.isLink(file)) {
>>             if (fsv.getLinkLocation(file) == null) {
>>                 throw new RuntimeException();
>>             }
>>         } else {
>>             if (fsv.getLinkLocation(file) != null) {
>>                 throw new RuntimeException();
>>             }
>>         }
>>
> The code will not compile because FileNotFoundException is not caught.
>
> The logic of getLinkLocation() is pretty simple:
> - If the file is not a link, it refers to nothing and the method 
> returns nothing (null).
> - If the file is a link, it refers to an another file but in case the 
> another file cannot be found in the FS the FileNotFoundException is 
> thrown.
>> There are some checks in the code related to "C:\pagefile.sys" and 
>> "C:\Winnt\Profiles\joe\history\History.IE5" can you double check how 
>> the new methods will work with these files.
> This situations are only possible for the listFiles() return. If it 
> happens for a File object then it is a real error which should be 
> reported.
>>
>>
>>> Your point that there is inconsistency sounds a bit odd.  If this
>>> reasoning we can not add a new method which throws an exception to the
>>> class not having methods throwing this exception. How methods of the 
>>> FSV
>>> class related to different aspects of the file system can be
>>> inconsistent? They are simply not related to each other.
>>
>>
>