Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [platform-ui-dev] Fwd: Generification of the JFace TreeViewer

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I've uploaded a new patch to gerrit:
https://git.eclipse.org/r/#/c/15805/

I changed the getFilteredChildren and also the getRawChildren methods
back to accept Object's as
parent parameter. This expects that parent parameter of the
getRawChildren method is from the type I for flat viewers, otherwise
this method has to be overwritten like in the AbstractTreeViewer.

@Markus
Thanks for your help, but why are you not convinced that the viewer
framework will be generified at all? Besides the implementation, I
don't see there any downsides.
The piecewise implementation was based on a decision we made, to get
smaller code reviews. But I think I understand your point, the used
types are like a chain. If one piece of the chain is broken/wrong this
has a big impact on the whole chain. So it is harder to work piece by
piece.

To the hierarchical viewers:
Of course with the java generics it's not possible to define types for
each level, but it is still possible to use Object as element type.
There are also data structures which depend on treeelements of a
homogeneous type. For this cases the use of generics at hierarchical
viewers is, in my eyes, reasonable.

Thanks and Regards,
Hendrik


Am 23.08.2013 13:03, schrieb Markus Keller:
> I didn't make a concrete proposal -- I just wanted to point out
> that I'd rather see some unchecked (but safe) casts than a big
> restructuring of viewer framework methods.
> 
> To be honest, I'm not convinced that the viewer framework should be
>  generified at all. Adding generics to a framework cannot be done 
> piecewise, but must be done in bigger chunks of related compilation
> units. One outcome of your project could very well be the
> conclusion that it's better not to generify most of the viewers
> package.
> 
> As you've already seen in StructuredViewer#refresh(Object), there
> are a few methods in the framework that can take either the
> input/root element or a child element. The
> StructuredViewer#getFilteredChildren(Object) method is another API
> like 'refresh' that is expected to work with any kind of parent
> element that can have children.
> 
> For flat viewers like list and table viewers, you can survive the
> first few levels of generification by declaring E[]
> getFilteredChildren(I parent) , but this breaks down for
> hierarchical viewers. To obey existing API contracts, the 'parent'
> parameter of the get*Children methods must stay Object. It could
> maybe take an 'I' in list and table viewers, but not in general
> StructuredViewers.
> 
> Furthermore, for hierarchical viewers you cannot add separate type
>  arguments for each level of the tree, so the gain in type safety
> would not be big for tree viewers anyway.
> 
> HTH, Markus
> 
> 
> Hendrik Still <hendrik.still@xxxxxxxxx> wrote on 2013-08-23
> 10:35:47: Thanks for the answer. I'm currently fixing some warnings
> in the code and will upload it later as new review. I'll also try
> to apply your suggestion. So your idea is to let the signature
> getFilteredChildren(I parent) as it is and cast the
> "elementOrTreePath" variables to I(the StructuredViewer expects an
> E) to satisfy the type system? I've already tested this, but I
> wasn't sure if this solution will confuse some people.
> 
> Regards, Hendrik
> 
> Am 22.08.2013 16:11, schrieb Markus Keller:
>>>> I don't have a good solution and the Gerrit link doesn't work
>>>> for me (page not found or no permission to view).
>>>> 
>>>> But I just wanted to point out that AbstractTreeViewer is
>>>> API, and although TreeViewer is marked as @noextend, we know
>>>> that many clients had to extend it to solve performance
>>>> problems. Therefore, I'd be very reluctant to change call
>>>> sequences or add more methods that are overridden by
>>>> subclasses.
>>>> 
>>>> It's probably better to use some unchecked casts to E to
>>>> satisfy the type system, but still funnel TreePath objects
>>>> through methods that should only operate on Es. However, this
>>>> can only work as long as you're sure that the erasure of such
>>>> method still stays Object in the signature. You have to do
>>>> some real-life experiments to be sure that casts in bridge
>>>> methods don't hit you at run time.
>>>> 
>>>> Markus
>>>> 
>>>> Hendrik Still <hendrik.still@xxxxxxxxx> wrote on 2013-08-20 
>>>> 15:41:12:
>>>> 
>>>>> Hi,
>>>>> 
>>>>> I'm currently adding generics support to the JFace
>>>>> TreeViewer, but I'm running in to some problems and I
>>>>> thought you could help me with that.
>>>>> 
>>>>> Unlike the other Viewers, with flat data structures like
>>>>> the TableViewer, the TreeViewer has to handle (of course)
>>>>> tree like data structures.
>>>>> 
>>>>> For the viewers in general we decided to have two type
>>>>> parameters for every viewer. E for an single element the
>>>>> viewer should display and I for the input you set with the
>>>>> setInput method. For example you are able to type a
>>>>> TableViewer with TrableViewer<Person,MyOwnListOfPersons>
>>>>> This type parameters are also applicable to the TreeViewer,
>>>>> but the tree structure and the internal implementation of
>>>>> the viewer does not allow a stronger typing of method
>>>>> parameters like it is done in the other viewers.
>>>>> 
>>>>> For example the method isExpandable(Object
>>>>> elementOrTreePath) in the AbstractTreeViewer class. This
>>>>> method handles elements (typed as E), the input (typed as
>>>>> I) and additional to that a TreePath(typed as TreePath) for
>>>>> the ITreePathContentProvider. The handling for the
>>>>> different types are managed at runtime. Also this works
>>>>> fine with generics, the problem is the call of the 
>>>>> getFilteredChildren(I parent) method from the 
>>>>> superclass(StructuredViewer). The getFilteredChildren
>>>>> method is not only called with as I typed parameters, but
>>>>> also with E and TreePath typed parameters. The signature of
>>>>> the getFilteredChildren method is correct for all viewers,
>>>>> except for TreeViewer.
>>>>> 
>>>>> After consulting my mentor Lars Vogel, my first try was to 
>>>>> override the getFilteredChildren method in the
>>>>> AbstractTreeViewer class with the signature
>>>>> getFilteredChildren(Object parent), which called the
>>>>> getRawChildren method of the AbstractTreeViewer. But to
>>>>> still execute the filters which could not be accessed from 
>>>>> the AbstractTreeViewer, I had to outsource the filter 
>>>>> functionality to another protected method (internalFilter)
>>>>> which can be called from the subclass.
>>>>> 
>>>>> You can see my currently running changes at 
>>>>> https://git.eclipse.org/r/#/c/15647/
>>>>> 
>>>>> So what do you think? Is this a acceptable solution for
>>>>> the issue?
>>>>> 
>>>>> Regards, Hendrik
>>>> 
>>>> _______________________________________________
>>>> platform-ui-dev mailing list platform-ui-dev@xxxxxxxxxxx 
>>>> https://dev.eclipse.org/mailman/listinfo/platform-ui-dev
>>>> 
> 
>> _______________________________________________ platform-ui-dev
>> mailing list platform-ui-dev@xxxxxxxxxxx 
>> https://dev.eclipse.org/mailman/listinfo/platform-ui-dev
>> 
> 
> _______________________________________________ platform-ui-dev
> mailing list platform-ui-dev@xxxxxxxxxxx 
> https://dev.eclipse.org/mailman/listinfo/platform-ui-dev
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQEcBAEBAgAGBQJSF2T8AAoJECjZOs4dIxisdzwIAJ056carjdwWzB7Wa6vRCEEM
QHgbOMZ7l+0b2d0dmkFL6yKnTytpI71tj1u4Jqde0n/JcoJLJhe0CE/zlteWupne
ly412dogHHepZezkQ+0J85YvwJSHegz2fBaVLQ3l/faac8NCF2+8b0SmjRKSfyjt
zdU0wNYO+8g2mezEf9Pg3E3C8UyURKlCclJaSj5ZfMCdNyO9blLQzXC2aLsoJPw1
1yy6dU705dTL/OclbKjE79BEuwizDSoxKMDCfvno8RBAVMctfw4LxIPScdqyCXhK
WLLu/oqy2NSBlSCJ6gbyKIZqGQxI9blqy/RGs9CW+hdaQRM4yA6T1gjfzUEPZnU=
=zNsQ
-----END PGP SIGNATURE-----


Back to the top