RFR: 8246592: Simplify checking of boolean file attributes

Roger Riggs Roger.Riggs at oracle.com
Fri Jun 5 09:23:57 UTC 2020


Hi Claes,

Also, since the requested attribute is a bit mask the test should be
that all of the attributes are present, not just some, iIt might prevent 
a mistake down the line.
Probably the method name and argument names should be plural.

Line: 120: should be

119 public boolean hasBooleanAttributes(File f, int attributes) { 120 
return (getBooleanAttributes(f) & attributes) == attributes; 121 }

Thanks, Roger


On 6/4/20 2:24 PM, Claes Redestad wrote:
> Hi Roger,
>
> On 2020-06-04 18:05, Roger Riggs wrote:
>> Hi Claes,
>>
>> The code looks ok but I would rename the new method to 
>> 'hasBooleanAttribute'
>> since it is not returning the attribute but testing for it.
>
> good suggestion:
>
> http://cr.openjdk.java.net/~redestad/8246592/open.01/
>
>>
>> I think you can leave the 'public' modifiers to a separate issue.
>
> Ok.
>
> /Claes
>
>>
>> Regards, Roger
>>
>>
>>
>>
>> On 6/4/20 9:41 AM, Claes Redestad wrote:
>>>
>>>
>>> On 2020-06-04 15:26, Alan Bateman wrote:
>>>> On 04/06/2020 14:22, Roger Riggs wrote:
>>>>> Hi Claes,
>>>>>
>>>>> Not a review...
>>>>>
>>>>> You'll need a CSR for the API change.
>>>>>
>>>>> FileSystem.java: You'll need proper javadoc for the new 
>>>>> getBooleanAttribute method.
>>>> FileSystem is not a public class so it's not actually an API 
>>>> change. I agree the public modifier is confusing, should probably 
>>>> be dropped from all these methods.
>>>
>>> If you prefer I can strip out all the public modifiers. Or do that as a
>>> follow-up.
>>>
>>> /Claes
>>



More information about the core-libs-dev mailing list