<Swing Dev> [9] Review request for 7020860: BasicTreeUI contains getters/setters with unclear spec

[Phil Race]

      /**
-     * Sets the {@code TreeCellRenderer} to {@code tcr}. This invokes
-     * {@code updateRenderer}.
+     * Called when the {@code cellRenderer} property is changed in
+     * the drawn tree component.
       *
-     * @param tcr the new value
+     * @param tcr the new value of the {@code cellRenderer} property
       */
      protected void setCellRenderer(TreeCellRenderer tcr) {
          completeEditing();
@@ -468,10 +471,10 @@
      }

why did you remove the reference to updateRenderer() ? Was it wrong ?
Also "Called when ... is changed"  occurs a few times in this webrev .. but changed by what ?


      /**
-     * Returns the row height.
+     * Returns the height of each row in the drawn tree component. If the
+     * returned value is less than or equal to 0 the height for each row is
+     * determined by the renderer.
       *
-     * @return the row height
+     * @return the height of each row, in pixels
       */
      protected int getRowHeight() {
          return (tree == null) ? -1 : tree.getRowHeight();
      }

Seems like it also returns -1 if there is no tree .. I don't see how the
spec. words account for that.


      /**
-     * Returns {@code true} if the tree root is visible.
+     * Returns whether the root node of the drawn tree component is displayable.
       *
-     * @return {@code true} if the tree root is visible
+     * @return {@code true} if the root node of the tree is displayed
       */
      protected boolean isRootVisible() {
          return (tree != null) ? tree.isRootVisible() : false;
      }
  
The API says "visible". Why change to "displyable", which usually is reserved
to mean whether something that has a native peer ?
Maybe some clarification of what "visible" means is in order.

-phil.



On 06/21/2016 04:55 AM, Alexandr Scherbatiy wrote:
> On 6/14/2016 5:37 PM, Semyon Sadetsky wrote:
>> Hello,
>>
>> Please review fix for JDK9:
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-7020860
>>
>> webrev: http://cr.openjdk.java.net/~ssadetsky/7020860/webrev.00/
>>
>> The methods mentioned in JIRA are not getters/setters according to 
>> JavaBeans specs, nevertheless some of them got wrong javadocs. The 
>> proposed fix improves the situation.
>
>    I am not sure about the phrase "that is rendering each cell" in
>
>  474      * Returns the current instance of the {@link 
> TreeCellRenderer} that is
>  475      * rendering each cell.
>
>   Otherwise, the fix looks good to me.
>
>   Thanks,
>   Alexandr.
>
>
>>
>> --Semyon
>>
>