RFR: 8322964: Optimize performance of CSS selector matching [v12]
Michael Strauß
mstrauss at openjdk.org
Tue May 28 11:28:12 UTC 2024
On Tue, 28 May 2024 11:01:21 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/FixedCapacitySet.java line 419:
>>
>>> 417: */
>>> 418:
>>> 419: assert elements.length > requestedCapacity : "must have more buckets than capacity";
>>
>> This just asserts that the previous three lines of code were correct. Seems a bit excessive to me, if you really need this tested, I'd prefer a unit test.
>
> True, but I think it's a lot easier to follow than the logic before it. I see it a bit more as a hint to future maintainers (which might be me) that there are assumptions being made here that code relies on.
I understand that, my objection is only with the `assert` keyword. I don't like test code in production code (which this is, it verifies that an assumption holds). I think only unit tests should contain test code, but it's a minor issue and I'll reapprove as-is.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1316#discussion_r1617055213
More information about the openjfx-dev
mailing list