Bug 179397 - Friendlier and non-type-specific way to set columns of OutlineView
Friendlier and non-type-specific way to set columns of OutlineView
Status: VERIFIED FIXED
Product: platform
Classification: Unclassified
Component: Explorer
6.x
All All
: P3 (vote)
: 6.x
Assigned To: _ tboudreau
issues@platform
: API, API_REVIEW_FAST
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-01-11 15:50 UTC by _ tboudreau
Modified: 2010-01-24 08:42 UTC (History)
1 user (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
Module which demonstrates the problem (17.37 KB, application/octet-stream)
2010-01-11 15:50 UTC, _ tboudreau
Details
Patch w/ tests & api change notes (17.06 KB, text/plain)
2010-01-11 18:31 UTC, _ tboudreau
Details
Considerably shorter and clearer version of the original module using this patch (16.20 KB, application/zip)
2010-01-11 18:41 UTC, _ tboudreau
Details

Note You need to log in before you can comment on or make changes to this bug.
Description _ tboudreau 2010-01-11 15:50:20 UTC
Created attachment 93196 [details]
Module which demonstrates the problem

For direct compatibility with TreeTableView, we retained the setProperties(Property[]) method for assigning the >0 column contents.

However, this results in two very awkward things:

1. If you don't have a Node yet, you need to write a subclass of PropertySupport.ReadOnly, just to pass the property name and type in - very non-obvious and wasteful of classes.

2. There is no practical reason why a column must contain only objects of the same type, yet exact type matching between the prototype property and the real property is enforced.  

Say you have a Node which a property which is an Enum;  and the Node has another property that will determine what concrete subclass of Enum will be used.  The property sheet will handle this fine and display the new property.  In the OutlineView, the column will be blank.

3.  Type matching, if necessary at all, ought to handle supertypes.  If I need a prototype property, it ought to be possible to create a prototype property with its value set to Object as a wildcard.  In fact, this is not possible - so you cannot display a property in the UI unless the UI code knows the *exact* type (not, say, an API-level supertype) of the class.  This encourages more coupling than necessary.

I suggest that setProperties() be deprecated, and a simpler
public void setPropertyNames(String...)
be used as a replacement.

I'm attaching a module which demonstrates the problem.  Note two things:
 - The listening on the root node in the TopComponent should not be necessary
 - The prototype property class should not be necessary (in this case, in fact, it isn't;  in the real world, often you don't have the root node when constructing the UI)
 - A single PropertySupport.ReadWrite<T extends Enum> class should be usable for both nested Property subclasses in MyNode
Comment 1 _ tboudreau 2010-01-11 18:31:38 UTC
Created attachment 93202 [details]
Patch w/ tests & api change notes

The attached patch adds the following mutator methods for the set of columns:

public void addPropertyColumn(String name, String displayName)
public void removePropertyColumn(String name)
public void setPropertyColumns(String... namesAndDisplayNames)

Under the hood, these use an internal Property subclass which does its equals() matching only using getName(), to avoid a common class of problem (prototype property value type not an exact match for real property).

I believe this addresses all of the problems.  Admittedly, passing an array of alternating programmatic name, display name, programmatic name, display name in setPropertyColumns is a little ugly, I think less so than adding yet another "property descriptor thing" to the API.
Comment 2 _ tboudreau 2010-01-11 18:41:50 UTC
Created attachment 93203 [details]
Considerably shorter and clearer version of the original module using this patch
Comment 3 _ tboudreau 2010-01-11 18:49:06 UTC
BTW, given further thought, points 2. and 3. aren't quite valid, since if you override equals() and hashCode() on your prototype property subclass to only test the property name, it will have the expected effect.  Nonetheless, you need to know to do this, and why are we making people write Property subclasses just to aggregate a name and display name and possibly a type, anyway?
Comment 4 _ tboudreau 2010-01-20 11:44:58 UTC
No objections during review period, implementing.

Fixed in main/ a183d81ffa42
Comment 5 Jaroslav Tulach 2010-01-21 10:46:38 UTC
Just yesterday, while struggling with OutlineView, I complained to myself who could design API with such a poor usability (me and Jano R. did it for TreeTableView). Thanks for this improvement, it makes my code nicer a easier to understand:
http://hg.netbeans.org/main-silver?cmd=changeset;node=505d5935f135
Comment 6 Quality Engineering 2010-01-24 08:42:22 UTC
Integrated into 'main-golden', will be available in build *201001240200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/a183d81ffa42
User: Tim Boudreau <tboudreau@netbeans.org>
Log: #179397 - eliminate need for prototype property objects to set columns of OutlineView


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo