On Fri, 10 Feb 2023 15:02:22 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org> wrote:
>> SwingWorker done() method [spec >> ](https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/javax/swing/SwingWorker.java#L452) >> says "Executed on the Event Dispatch Thread after the doInBackground method >> is finished" >> but there's no mechanism in place to honor that claim. >> The >> [spec](https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/javax/swing/SwingWorker.java#L289) >> also says the state should be DONE after doInBackground() returns which is >> also not done. >> >> Modified the code to honour the specification. > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Space rectify Changes requested by aivanov (Reviewer). test/jdk/javax/swing/SwingWorker/TestDoneBeforeDoInBackground.java line 90: > 88: // property change to StateValue.STARTED > 89: if (doInBackgroundFinished.get() > 90: && worker.getState() == > SwingWorker.StateValue.STARTED) { Suggestion: if (doInBackgroundStarted.get() && worker.getState() == SwingWorker.StateValue.STARTED) { Align the condition to the error message. test/jdk/javax/swing/SwingWorker/TestDoneBeforeDoInBackground.java line 99: > 97: // before doInBackground started running > 98: if (doInBackgroundStarted.get() > 99: && worker.getState() == > SwingWorker.StateValue.STARTED) { Suggestion: if (doInBackgroundFinished.get() && worker.getState() == SwingWorker.StateValue.STARTED) { Align the condition to the error message. test/jdk/javax/swing/SwingWorker/TestDoneBeforeDoInBackground.java line 109: > 107: // property change to StateValue.DONE > 108: if (worker.getState() != SwingWorker.StateValue.DONE > 109: && doInBackgroundFinished.get()) { Suggestion: if (worker.getState() != SwingWorker.StateValue.DONE && doInBackgroundFinished.get()) { This should also use four-space indentation on the continuation line for consistency with other cases. test/jdk/javax/swing/SwingWorker/TestDoneBeforeDoInBackground.java line 135: > 133: && doInBackgroundFinished.get()) { > 134: throw new RuntimeException("doInBackground is finished " + > 135: "but State is not DONE"); Suggestion: if (worker.getState() != SwingWorker.StateValue.DONE && doInBackgroundFinished.get()) { throw new RuntimeException("doInBackground is finished " + "but State is not DONE"); Throw shouldn't be indented by additional two spaces. With the `&&` at the start of the line, it is rather clear where condition ends. Additionally, `throw` cannot be part of the condition. It's a catch with `if` statement because its condition is always indented by four spaces. If required, a blank line could be added in the body of `if` to separate the condition visually; continuation line could use 8-space indentation. ------------- PR: https://git.openjdk.org/jdk/pull/11940
