TreeTableCell: Misnamed / inconsistent methods for row and column properties

[Kevin Rushforth]

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