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

Marius Hanl mhanl at openjdk.org
Thu Feb 15 12:16:03 UTC 2024


On Wed, 14 Feb 2024 20:57:14 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> 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)

yes, but other cells have the exact same pattern, although it is not needed. And technically speaking, this is not even needed in `TreeTableCell`, tbh I never even needed a labeled if ever. 😄 
But sure, can also change.

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

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


More information about the openjfx-dev mailing list