RFR: 8246592: Simplify checking of boolean file attributes

[Claes Redestad]

Hi Roger,

yes, things were a bit inconsistent whether this is checking all or one.
I went with all (even though all usage only asks for one) and updated
to use plural consistently.

/Claes

On 2020-06-05 11:23, Roger Riggs wrote:
> 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
>>>
>