On Sat, 27 Jan 2024 21:56:59 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/javafx/animation/Animation.java line
>> 903:
>>
>>> 901: }
>>> 902:
>>> 903: private void playFromOnFxThread(String cuePoint) {
>>
>> I take that adding these private `xxOnFxThread` methods is convenient so
>> they can be passed easily to the runnable in `Utils::runOnFxThread`, or
>> accessed from other subclasses.
>>
>> However, I'm not sure about the method naming `OnFxThread`, as it might
>> imply that what it does is already done on the FX thread, therefore not
>> needing to be wrapped up by `runOnFxThread`.
>>
>> Does `runXX` (i.e. `runPlayFrom`) make more sense? In any case, I'd be fine
>> with the current proposal as is.
>
>> However, I'm not sure about the method naming `OnFxThread`, as it might
>> imply that what it does is already done on the FX thread, therefore not
>> needing to be wrapped up by `runOnFxThread`.
>
> I understand what you mean, but the way I see it is that they do already run
> on the FX thread because that's the only reason they exist. I could use
> something like `runMustBeOnFxThread` or `runOnlyFromFxThread` if that's
> better.
> Note that there are already `do___` methods (bad names IMO), and adding
> `run___` might be confusing.
Probably you won't like `xxImpl` either?
I'm not sure we should start adding `runMustBeOnFxThread` to every method name
out there that should run on the FX thread. In this case, maybe we could simply
add a small javadoc with a comment, and that's it. Neither will enforce a real
check about that.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1352#discussion_r1469290172
