Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [cross-project-issues-dev] JFace Generics

Comments inline.

On 30.08.2013, at 10:23, Aleksandar Kurtakov wrote:

> ----- Original Message -----
>> From: "Ed Merks" <ed.merks@xxxxxxxxx>
>> To: cross-project-issues-dev@xxxxxxxxxxx
>> Sent: Friday, August 30, 2013 10:54:45 AM
>> Subject: Re: [cross-project-issues-dev] JFace Generics
>> Guys,
> Comments inlined.
>> Contributions are a wonderful thing in principle, and should be strongly
>> encouraged in practice, but using that as an argument to deflect technical
>> criticism about the merit of generifying the JFace APIs is highly suspect.
> Technical problems were, are and will always be here. I'm not trying to deflect technical criticism but as there are people interested in see value in JFace generification their contribution shouldn't be rejected because a technical problem is here. The "<? extends E>" type of suggestions/bugs is exactly what needs to happen, help people with your expertise to get better codebase. If this things has been developed in separate branch I would dare to say that noone would have looked at it and we would have been having same discussion at the time of branch merge, way later in the release cycle and with way more changes done in not the best way. 
>> Of course everyone is more than welcome to deflect the technical criticism
>> with technical arguments to contrary, as some have done. It's an open
>> community and debate is more than healthy. It's already clear from the
>> debate how poorly generics and arrays play together, and how unfortunately
>> poorly that's understood. I.e., sorry but you cannot do "new E[size]", you
>> just can't create a typed array without knowing the java.lang.Class. That's
>> a major problem. Note that if you have a raw list, and you know/believe it
>> to contain only elements of some specific type Foo, you can cast it to
>> List<Foo> and use it. But even if you know an IStructuredContentProvider
>> only works with instances of Foo and only returns Foo elements, casting it
>> to IStructuredContentProvider<Foo> and working with it, you'll likely find
>> that the Object[] result of getElements isn't a Foo[] and doesn't work.
>> E.g.,
>> might be assumed to conform to ITreeContentProvider<IDiffElement,
>> IDiffContainer> but its currently implementation doesn't conform until it's
>> actually reimplemented "properly".
>> I should emphasize too that I'm not so naive to argue there is zero merit to
>> generification, but let's face the facts, these APIs, rife with Object[]
>> instead of List<...>. As such, it's effectively stone age design when viewed
>> in that light. An attempt to turn this into a modern generic container
>> framework in face of the widespread use of array types which will never play
>> well with generics seems to me at least somewhat misdirected. For example,
>> another scenario where the changes will be problematic is in
>> org.eclipse.jface.viewers.ITreePathContentProvider. That generification will
>> have less than no value unless TreePath itself is generified: instance of
>> checks on the path segments will remain if TreePath isn't a generic
>> container, yet the return types of the methods are of the content provider
>> are constrained, and those must be respected via actual changes in the code,
>> not just by sprinkling the appropriate <> here and there. Addressing the
>> generification of TreePath involves handling
>> org.eclipse.jface.viewers.TreePath.TreePath(Object[]), and there are those
>> darned arrays again. This can't be changed to E[] can it? Generic code
>> creates these arrays to create the TreePaths, and such code can't create an
>> array of E. So the end result will be no static type safety in creating the
>> instance of TreePath if it remains Object[], and hence failures will happen
>> later on access to the segments, in bridge methods without source. Is there
>> improvement in this?
> Well, personally, I think it's time for the platform to do even one more step further - provide collections based API and deprecate the arrays one. That doesn't even have to be done at once as deprecated methods can stay for long long time. Paving the road ahead is badly needed and nothing should be seen as "unchangeable" in the long run. 

I doubt that there's anything really unchangeable, but the success of the platforms also stems from the fact of long term backwards compatibility. That's an asset we should not trade in for anything.

>> It's totally frustrating to me that the platform hasn't yet address the last
>> round of generification on Java collections, e.g.,
>> org.eclipse.jface.viewers.IStructuredSelection.iterator() is still raw and
>> more annoyingly, so is
>> org.eclipse.core.runtime.IAdaptable.getAdapter(Class). Yet now the goal is
>> to introduce more such changes that will of course remain unaddressed in the
>> entire huge ecosystem of downstream APIs. How long into the future will that
>> disparity continue to exist? Will that make Eclipse look modern? Does that
>> address the "why I hate Eclipse" mentality of the actual users of Eclipse.
>> Does it introduce interesting powerful new functionality?
> There are people interested in doing these changes, if there are people interested in doing what you mentioned it would happen. It's FOSS after all. With Eclipse becoming a community driven project that's how goals are set. It might not fix the "why I hate Eclipse" but it will fix the "why I hate contributing to projects" for many people capable of doing it. And this is the first step to be done, if the project can not attract the people capable of maintaining and improving it, it will never fix the problems for users.
> I agree with many of your technical comments regarding the current state but I totally disagree with throw it away in the corner where noone will see it. We are not at M2 yet, further exposing the changes will make people give feedback so the API can get way better by M6 there are 6+ months.

If we can't even identify a technically feasible, backwards compatible way for API implementors and API consumers, I doubt that there's realistic chance that the APIs will mature in a reasonable time-frame, is there? Shouldn't we remove at least the major blockers before such a huge change is rolled out to everyone?
Seriously, is there a rough estimation how much effort would go into this generification compared to how many 'casts' will actually be removable from the code base? Does the API even become easier to use and implement or is only more cluncy, because e.g. children in a tree viewer are usually heterogenious?
We should really try to find an answer to the technical challenges before we find us in a situation that only allows to rollback that stuff. If generification of an existing API and codebase would be straigtforward, how comes that there is no automated support for that that works without manual interaction? So especially when arrays come into play, it's not just a matter of adding <E> and E[] on the surface of interfaces. That will never blend. It can't. 

I'm the last one who wants to turn contributions down, but we have to keep in mind the gazillion lines of code out there that may be affected by them compared to the impact that an end user will notice.

Just thinking,

> Alexander Kurtakov
> Red Hat Eclipse team
>> Worse still for EMF specifically, is that we're maintaining compatibility
>> with Eclipse 3.6, so how to address changes that require compiling against
>> Eclipse 4.4's APIs? I suppose we could build against those new APIs and test
>> against a 3.6 runtime, but does that really work in the face of the bridge
>> methods generated when using these APIs? Perhaps it does, and given that I
>> doubt there is a single place in the EMF code base that can actually make
>> use of anything other than <Object> and <?>, maybe there will be no bridge
>> methods, and we'll only end up with more noise in the code...
>> Working in a side branch would seem far better. Then the full impact can be
>> assessed before we're all fully impacted, and we can actually assess whether
>> this all pans out, rather than the current approach that simply assumes it
>> will all pan out reasonably well. It's an assumption that is questionable.
>> Regards,
>> Ed
>> On 29/08/2013 10:20 PM, Doug Schaefer wrote:
>> On 13-08-29 3:49 PM, "Aleksandar Kurtakov" <akurtako@xxxxxxxxxx> wrote:
>> ----- Original Message -----
>> From: "John Arthorne" <John_Arthorne@xxxxxxxxxx> To: "Cross project issues"
>> <cross-project-issues-dev@xxxxxxxxxxx> Sent: Thursday, August 29, 2013
>> 7:37:17 PM
>> Subject: Re: [cross-project-issues-dev] JFace Generics
>> You raise good arguments as always Ed. I will attempt to summarize and
>> respond to some of the comments raised in this thread.
>> Q: Should JFace be generified at all.
>> As with most library generifications there are pros and cons that we can
>> probably debate all day. I still think on balance it will be a net
>> benefit
>> for consumers. Some will not have homogeneous element types and will be
>> stuck with Object, and therefore be no better or worse off than before.
>> TreeViewer is the worst case, although I think it would be difficult and
>> awkward to generify some viewer types but not others. My experience with
>> most generification is that 90% of usage is adapted relatively
>> painlessly
>> and eliminates a tonne of casting, resulting in cleaner code. There are
>> always some rough edges, especially where arrays are found, and
>> suppressing
>> warnings is sometimes necessary. There is certainly work involved for
>> clients to adopt it, although I don't find that alone a compelling
>> argument
>> to do nothing at all. A good place to continue this general discussion
>> would
>> be the master bug report:
>> Q: Should we release
>> incomplete work to a milestone
>> I'm torn on this one. Certainly in the past our project adopted a "we
>> won't
>> accept your contribution until it is perfect" approach. There is no
>> doubt
>> this benefits consumers, but I think it has strongly discouraged all
>> but the
>> most die-hard contributors to the project. Given the number of remaining
>> active contributors, I strongly believe we need to find ways to make
>> progress using the contributors we have, at the rate they are capable of
>> contributing.
>> Huge +1000 here. Eclipse platform is one of the projects that's hardest
>> to get things into and I couldn't say that it's so much better than
>> others like Gnome or KDE for example.
>> The longer changes are delayed the more disruptive changes become. Even
>> if certain things might be disruptive now this is the price to be paid
>> for neglecting majority of contributors for years.
>> Please keep opening the project - rejecting contributions is the road to
>> oblivion. The question shouldn't be whether it benefits all API users but
>> whether it breaks stuff. If it doesn't and if there is contributor
>> willing to spend time on it he/she should be encouraged to contribute
>> even if it will benefit single digit percentage of users. Growing
>> contributors never starts with huge things, it works by piling stuff on
>> and getting contributors used to workflow and guidelines in the project.
>> Same here. You have to ask yourself whether rejecting contributions and
>> evolution of the APIs has led to the reduced number of active contributors
>> we have. I know it's more complicated than that, but please don't discount
>> it.
>> Alexander Kurtakov
>> Red Hat Eclipse team
>> Undoubtedly this is a slower and bumpier road, but I think the
>> risk is better than the alternative of stagnating because there are no
>> contributors who can keep up with the old pace expected/demanded by the
>> community. Certainly if there was risk of data loss or severe problems
>> with
>> tool usability, we would have avoided releasing in a milestone. In this
>> case
>> the worst case is compiler warnings which can be disabled if desired. I
>> understand there are some who consider warnings to be catastrophically
>> bad,
>> but the community compiling against our milestones is a tiny fraction
>> of the
>> overall community. I believe all changes released were binary
>> compatible and
>> not changing behaviour.
>> In the end I admit we got this one wrong. We misjudged the rate of
>> progress
>> on the work, and many committers being on holiday bogged down the review
>> process. We should have kept the work in a branch and waited until at
>> least
>> after M1 to release. Some damage is already done, but we are now
>> investigating moving the work into a branch. This will actually
>> introduce a
>> much worse problem for any client that had reacted to changes already,
>> since
>> it will result in compile errors. We'll send a separate warning note
>> about
>> this once we are sure on this path, so it doesn't get lost in this
>> long-winded message.
>> Q: I have some specific technical concerns about the approach taken so
>> far.
>> This is excellent feedback and I'm sure it will be incorporated into
>> the work
>> going forward. I wonder if we would have had such feedback if we hadn't
>> released anything to master ;)
>> Q: Your communication sucks.
>> Yes it does. I think normally we wait until things are closer to
>> completion
>> before making big announcements. It is quite often that we have feature
>> work
>> in progress appearing in a milestone, that we hold off on announcing
>> until a
>> future milestone when it is more polished. In this case because the
>> interim
>> state already had significant source level impact this should have been
>> communicated more widely.
>> John
>> From: Ed Merks <ed.merks@xxxxxxxxx> To: cross-project-issues-dev@xxxxxxxxxxx
>> ,
>> Date: 08/29/2013 03:52 AM
>> Subject: Re: [cross-project-issues-dev] JFace Generics
>> Sent by: cross-project-issues-dev-bounces@xxxxxxxxxxx Hi,
>> It's difficult to avoid an emotional outburst at this type of thing. I'm
>> completely shocked that sweeping changes of this nature go unannounced
>> on
>> this mailing list. Sorry, a blog doesn't cut it...
>> It's clear the current state is woefully incomplete. I.e., we have
>> IStructuredContentProvider<E, I> but ITreeContentProvider is still raw.
>> Of
>> course it's clear that a tree rarely has uniformly typed children, so
>> what
>> is the plan for the completion of JFace's APIs? I question all this
>> being
>> committed to master in incremental stages like this...
>> EMF is a sea of warnings with these changes, and eliminating those is
>> weeks
>> of work; work I can't begin because the changes are incomplete. And of
>> course this affects all users of JFace, not just EMF, so the overall
>> impact
>> to the community is hard to calculate. The most disturbing part of all
>> this
>> is that I question whether it has even been well thought out. For
>> example,
>> the following change is highly disturbing:
>> public interface IStructuredContentProvider<E,I> extends
>> IContentProvider<I>
>> {
>> public E[] getElements(I inputElement);
>> }
>> Suppose I used it like this:
>> public class GenericContentProvider<E, I extends List<E>> implements
>> IStructuredContentProvider<E, I> {
>> @Override
>> public void dispose() {
>> }
>> @Override
>> public void inputChanged(Viewer<I> viewer, I oldInput, I newInput) {
>> }
>> @Override
>> public E[] getElements(I inputElement) {
>> return (E[])inputElement.toArray();
>> }
>> }
>> I.e., I have a generic content provider implementation class that for a
>> List<E> input returns the elements of that list. But there is a warning
>> in
>> the code for "E[])inputElement.toArray();" and it's not something one
>> can
>> blithely ignore. It will never be possible to create a generic subclass
>> of
>> IContentProvider that leaves E unsubstituted by a concrete
>> implementation
>> class, because it will never be possible to create an E[] array. If you
>> question this assertion, stop and ask yourself why
>> java.util.Collection.toArray() if of type Object[] and not of type E[]?
>> It's
>> because it would not be possible to implement generic Collection
>> implementation classes.
>> I can't emphasize enough how disturbing I find the approach being taken
>> here.
>> We're all familiar with using generics, but implementing generic classes
>> properly remains complex and tricky and what's being done in JFace
>> doesn't
>> just impact the use of generics, it forces us all to revisit our
>> implementation classes. For example, perhaps someone can explain how
>> org.eclipse.jface.viewers.ArrayContentProvider will be updated? Probably
>> just to "class ArrayContentProvider implements
>> IStructuredContentProvider<Object, Object>" I would imagine, but that's
>> not
>> terribly useful is it? I imagine the overall impact on the community is
>> to
>> make sweeping pointless changes of precisely this nature. But suppose I
>> even
>> have a nice implementation class where I wanted
>> IStructuredContentProvider<Foo, Bar>, my current implementation of
>> getElements is probably wrong and would need to change to return Foo[].
>> But
>> of what value is that? ContentProviders are generally just passed to a
>> generic viewer, which uses it in a context where the types don't
>> matter. So
>> what's the benefit?
>> Sorry to be so extreme in my opinion, but I would go further and argue
>> that
>> it's hard to imagine a significant set of scenarios where the new APIs
>> are
>> helpful even if this generic array issue wasn't just plain wrong or a
>> horribly bad idea... I could go on and on, but as I said, it's hard to
>> remain unemotional about this...
>> Regards,
>> Ed
>> On 29/08/2013 7:46 AM, Lars Vogel wrote:
>> Hi Eike,
>> this is a GSoC done by Hendrik (cc) and was announced on PlanetEclipse.
>> See .
>> John Arthone and I are the mentors for this project
>> The work is still ongoing so far the ComboViewer and the TableViewer
>> have
>> been converted as well several basic classes. We currently don't know
>> if we
>> can generify IStructuredSelection.
>> Input is very welcome, the umbrella bug is
>> and every code
>> change
>> is pushed to Gerrit. Look in
>>,n,z for
>> Hendrik
>> (so don't know how to narrow the query down to Hendrik only).
>> Best regards, Lars
>> 2013/8/29 Eike Stepper < stepper@xxxxxxxxxx >
>> Hi,
>> After updating my target platform to Luna I noticed that my workspace
>> is full
>> of raw type warnings caused by changes in JFace to generify its APIs,
>> for
>> example Viewer. But the changes look incomplete, e.g.,
>> ViewerDropAdapter.getViewer() is still a raw type. Can we expect more
>> such
>> changes, e.g., IStructuredSelection?
>> Has there been an announcement for these changes? Is there any advice
>> on how
>> to adjust our code?
>> What about other Eclipse APIs, such as IAdaptable.getAdapter(Class),
>> will it
>> be changed to <T> T getAdapter(Class<T>)?
>> Cheers
>> /Eike
>> ----
>> _______________________________________________
>> cross-project-issues-dev mailing list cross-project-issues-dev@xxxxxxxxxxx
>> _______________________________________________
>> cross-project-issues-dev mailing list cross-project-issues-dev@xxxxxxxxxxx
>> _______________________________________________
>> cross-project-issues-dev mailing list cross-project-issues-dev@xxxxxxxxxxx
>> _______________________________________________
>> cross-project-issues-dev mailing list cross-project-issues-dev@xxxxxxxxxxx
>> _______________________________________________
>> cross-project-issues-dev mailing list cross-project-issues-dev@xxxxxxxxxxx
>> _______________________________________________
>> cross-project-issues-dev mailing list cross-project-issues-dev@xxxxxxxxxxx
>> _______________________________________________
>> cross-project-issues-dev mailing list
>> cross-project-issues-dev@xxxxxxxxxxx
> _______________________________________________
> cross-project-issues-dev mailing list
> cross-project-issues-dev@xxxxxxxxxxx

Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail

Back to the top