On Wed, 16 Mar 2022 21:17:44 GMT, lawrence.andrews <d...@openjdk.java.net> 
wrote:

>> Following methods are covered in this testcase
>> getAccessibleAction()
>> getAccessibleActionCount()
>> doAccessibleAction(int direction) 
>> 
>> @shurymury
>
> lawrence.andrews has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Fixed review comments

Changes requested by shurailine (Committer).

test/jdk/javax/accessibility/JSlider/AccessibleAction/JSliderAccessibleAction.java
 line 74:

> 72:     private static volatile AtomicInteger currentJSliderValue;
> 73:     private static volatile AtomicInteger jSliderInitialValue;
> 74:     private static volatile AccessibleAction accessibleAction;

This field seem to be only accessed from the EDT, similar to UI components 
above, so why is it volatile and the components are not?

test/jdk/javax/accessibility/JSlider/AccessibleAction/JSliderAccessibleAction.java
 line 179:

> 177:                 validDecrementCountDownLatch = new CountDownLatch(1);
> 178:                 validIncrementCountDownLatch = new CountDownLatch(1);
> 179:                 currentJSliderValue = new AtomicInteger();

if this is a volatile, than perhaps you do not need an atomic?

test/jdk/javax/accessibility/JSlider/AccessibleAction/JSliderAccessibleAction.java
 line 265:

> 263:         robot.waitForIdle();
> 264:         final Point[] point = new Point[1];
> 265:         final Rectangle[] rect = new Rectangle[1];

finals here are redundant although do not hurt

test/jdk/javax/accessibility/JSlider/AccessibleAction/JSliderAccessibleAction.java
 line 282:

> 280:     public static void main(String[] args) throws InterruptedException,
> 281:             InvocationTargetException, AWTException {
> 282:         JSliderAccessibleAction jSliderAccessibleAction =

The class appears to have no state, why do you need an instance of it? Why not 
to make the methods static?

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

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

Reply via email to