On Sun, 28 Jun 2020 02:35:16 GMT, John Neffenger
<github.com+1413266+jgn...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/com/sun/glass/ui/Pixels.java line 194:
>>
>>> 193: public final Buffer getBuffer() {
>>> 194: assert this.bytes != null || this.ints != null;
>>> 195: return this.bytes != null ? this.bytes : this.ints;
>>
>> We should not use `assert` statements in the JavaFX runtime. If this is a
>> condition that needs to be checked, then use
>> an `if` test and throw an appropriate `RuntimeException` or an
>> `InternalError`.
>
> Thanks, Kevin. I saw `assert` used elsewhere (`EventLoop` in the same
> package, for example), and thought it was
> okay—even preferable—for these "should never occur" checks. Is that a general
> policy not to use `asserts` anywhere in
> JavaFX? Should I also remove the asserts I added to `HeadlessScreen` and
> `EPDScreen` in lieu of a unit test case?
I don't know whether it is documented, but we did at one point have a policy to
avoid them and removed a few elsewhere
in glass (which is where most of the usage was). The main reason is that when
running applications that want to use
assertion checking themselves it's too easy for a stray assert in a library to
cause problems if we don't test for it.
In case it really is an invariant, and nothing that could be provoked by API
calls (which I think this case is), then
checking explicitly seems best.
I wouldn't recommend removing any existing `assert` statements, but we could
file a follow-up issue to remove them
-------------
PR: https://git.openjdk.java.net/jfx/pull/255
