> i think to have just finished to add missing @Override in pivot > sources (i hope :-) ). >
Thanks! Can you resolve the JIRA ticket? > I've seen some small things: > - in TerraTreeViewSkin, there are a warning on two empty for, in > addVisibleNode() and in removeVisibleNodes() ... as seen before for > other warnings on while empty I think should be modified, for better > readability, and to avoid warnings ... > In those cases, there are comments right before the loops explaining what's going on, and they're just scanning loops (used to position an int variable). Can you suggest a different phrasing so we can see the options? > - in TerraTableViewHeaderSkin, i got this warning "Exporting > non-public type through public API" > protected SortIndicatorImage sortAscendingImage = new > SortIndicatorImage(SortDirection.ASCENDING); > protected SortIndicatorImage sortDescendingImage = new > SortIndicatorImage(SortDirection.DESCENDING); > changing them to private solve ... i make the change, right ? > I'd make the SortIndicatorImage class protected instead. > - there are many warnings on "Local variable hides a field" (in many > sources), and i know the the code works good also with this, but what > do you think on changing a little these variables, to avoid this ? > Using Best practices always help ... > I actually think it's ok to have local variables hide a field. Otherwise, you end up naming your local variables with contrived names, and in most (if not all) cases where we do this, our local variable is a view (or decorator, in design pattern terminology) of our field, so the naming parity is intentional. -T
