RFR: 8290310: ChangeListener events are incorrect or misleading when a nested change occurs [v13]

[John Hendrikx]

On Wed, 12 Mar 2025 01:52:16 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> They're useful during testing (when `-ea` is passed, which it generally is). They will help catch bugs if this code is ever refactored.  As for the current implementation, I'm confident that it will never trip these asserts.  It also serves as a bit of documentation.  I'm fine with removing these -- I placed them in separate methods to not mess up inlining of compiled code (asserts will count against the method length limit if placed directly in the method).  As it is now, they will be completely eliminated when not running with `-ea`.
>> 
>> As far as allowing goes -- FX is full of asserts already, so I guess its allowed?
>
> I don't mind personally, I just know some people "frown upon" `assert` in production code. @kevinrushforth can comment on it, but I don't foresee a problem.

I think I left these in because of the need to subclass these, and the subclasses are expected to work in a certain way and must be highly optimized.  Normally, I'd just make them hard checks (with `IllegalStateException` or `IllegalArgumentException`), but for optimal performance it is better to do it with asserts in this case (I don't want it to be too much slower than `ExpressionHelper`).  With the asserts, if you write any kind of test code for your subclasses, then it will fail quickly if you made a mistake.  As said, I'm confident these subclasses that we have now are implemented correctly, but who knows, in the future we may want to use similar code for ListChangeListeners or something...

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r1990423826