TreeTableCell: Misnamed / inconsistent methods for row and column properties

[Kevin Rushforth]

PR #575, which implements option 2, is now RFR.

-- Kevin

On 7/13/2021 7:58 AM, Kevin Rushforth wrote:
> Here is a Draft PR that implements option 2:
>
> https://github.com/openjdk/jfx/pull/575
>
> It also fixes two additional issues with the properties:
>
> * The docs for each property is on a private property field that does 
> have tree in the name, which results in no docs being generated for 
> either the tableRow or tableColumn property.
>
> * The implementation of the tableRowProperty() method returns the 
> writable property by mistake rather than the read-only property that 
> is specified by the method's return type.
>
> I will likely make it RFR tomorrow, and plan to target it to jfx17.
>
> -- Kevin
>
>
> On 7/12/2021 2:55 PM, Kevin Rushforth wrote:
>> In looking at this more closely, I now think that option 2 is the 
>> better approach for the following reasons:
>>
>> * TreeTablePosition extends TablePositionBase (which is also extended 
>> by TablePosition), and inherits a getTableColumn() method that is 
>> overridden with a covariant return type (using the type of the 
>> generic "TC" parameter) of TreeTableColumn.
>>
>> * Although not public API, TreeTableCellBehavior extends 
>> TableCellBehaviorBase ( which is also extended by TableCellBehavior), 
>> and inherits inherits a getTableColumn() method that is overridden 
>> with a covariant return type of TreeTableColumn.
>>
>> So option 2 is the one that will provide better consistency, in 
>> addition to being less intrusive.
>>
>> Thus, the only change I am proposing to the public API is: 
>> getTreeTableRow() -> getTableRow() -- which I will do by deprecating 
>> (not for removal) the existing method and adding getTableRow() to 
>> match the property name.
>>
>> -- Kevin
>>
>>
>> On 7/12/2021 10:22 AM, Kevin Rushforth wrote:
>>> While evaluating a javadoc fix [1] that removes spurious warnings 
>>> for missing comments on JavaFX property methods, I looked at the 
>>> remaining warnings, and discovered an inconsistency in the naming of 
>>> two of the properties in the TreeTableCell class. I filed 
>>> JDK-8270314 [2] to track this.
>>>
>>> Before I submit a PR, I wanted to solicit comments.
>>>
>>> In TreeTableCell, there is a mismatch between name of the following 
>>> property method vs the getter:
>>>
>>>   tableRowProperty()
>>>   getTreeTableRow()
>>>
>>> the get method has "Tree" in the name while the property method does 
>>> not.
>>>
>>> The corresponding methods for column are self-consistent, and are 
>>> named without the "tree" in the name:
>>>
>>>       tableColumnProperty()
>>>       getTableColumn()
>>>
>>> Given the type of the property -- TreeTableRow and TreeTableColumn, 
>>> respectively -- the properties should have "Tree" in the name, but 
>>> don't. Somewhat related is that the update methods for both row and 
>>> column are updateTreeTableColumn​ and updateTreeTableRow​ which do 
>>> have "TreeTable" in the name:
>>>
>>> There are two possible solutions that seem workable:
>>>
>>> Possible solutions:
>>>
>>> 1. Add Tree to all methods of the row and column properties
>>>
>>>   tableRowProperty() --> treeTableRowProperty()
>>>
>>>   tableColumnProperty() --> treeTableColumnProperty()
>>>   getTableColumn() --> getTreeTableColumn()
>>>
>>> This is the most consistent, but is slightly more intrusive in that 
>>> changes 3 of the 4 public methods of the row and column properties.
>>>
>>>
>>> 2. Remove Tree from all methods of these properties
>>>
>>>   getTreeTableRow() -> getTableRow()
>>>
>>> This is a less intrusive change that only affects one method. While 
>>> it makes the properties self-consistent and consistent with each 
>>> other, the names are not what would be expected given the return 
>>> type (TreeTableColumn and TreeTableRow), and are not consistent with 
>>> the update methods. Also, for applications that don't use the 
>>> property method, and only use the getter, it isn't any less change 
>>> than the the first option.
>>>
>>>
>>> I propose to go with option #1 for maximum consistency and further 
>>> propose to deprecate *not for removal* the misnamed methods.
>>>
>>>
>>> Comments?
>>>
>>> -- Kevin
>>>
>>> [1] https://github.com/openjdk/jdk/pull/4747
>>> [2] https://bugs.openjdk.java.net/browse/JDK-8270314
>>>
>>
>