RFR : JDK-8024352 - MBeanOperationInfo accepts any int value as "impact"

[Ujwal Vangapally]

Hi Roger,

kindly see my understanding below.


On 11/7/2017 9:10 PM, Roger Riggs wrote:
> Hi Ujwal,
>
> MBeanOperationInfo:163:
> Since the values are fixed, you could more concisely just compare 
> impact >=0 and impact <= UNKNOWN.
As there are only 4 constants, I thought it would be better to include 
them directly instead of depending on there values.

If it is a good practice to do it by comparing there values I will do it 
that way.
>
> 257/263:  I don't see a reason to change the toString in the default 
> case for getImpact().
>
As impact can't have any other value than 4 constants, we don't need the 
default case any more.
>
> A suggestion would be to introduce an Enum for the action values and 
> the corresponding new
> method; perhaps deprecating the current method (or not).
> The enum would use the same values as currently and so internally the 
> implementation does not
> change significantly.
Kindly clarify.
As it changes the constructor signature if we introduce a Enum,
but as we can solve the issue by throwing IAE, do we still need to 
introduce Enum and change method signature.

Thanks,
Ujwal.
>
> $.02, Roger
>
>
> On 11/7/2017 6:05 AM, Ujwal Vangapally wrote:
>> Kindly review the fix for bug below.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8024352
>>
>> webrev : 
>> http://cr.openjdk.java.net/~uvangapally/webrev/2017/8024352/webrev.00/
>>
>> Thanks,
>>
>> Ujwal.
>>
>>
>