RFR: JDK-8325402: TreeTableRow updateItem() does not check item with isItemChanged(..) unlike all other cell implementations

[Andy Goryachev]

On Wed, 14 Feb 2024 20:48:33 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableRow.java line 424:
>> 
>>> 422:                     // proceed with the code following this, so that we may
>>> 423:                     // still update references, listeners, etc as required.
>>> 424:                     break outer;
>> 
>> I understand neither the comment nor this `break outer;`: there is no code that follows `outer: if`, so we might as well `return`, right?
>
> Yes! I did it this way to be consistent with all other cell implementations. I really think that this should be refactored, I can also create a ticket for this. My preference would be to be consistent now, but change it for all cell implementations in one go.
> What is your opinion?

my opinion: the code is not broken per se.  Since this block is edited, I would have changed it just in this file to make the logic clearer (in other places, like TreeTableCell:685 it is functional because we have more code in line 692 and on).
I don't think another ticket is necessary because, technically speaking, the code is not broken.
(just my opinion)

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1360#discussion_r1490058786