Community
Participate
Working Groups
Build ID: I20070621-1340 The ColumnViewerEditorActivationListener is a fully abstract class, which forces implementors into extending the activation listener class. This seems unnecessary in Java when an interface would perform the same job whilst leaving implementors free to use a more natural class hierarchy. I realise that this can't easily be fixed now that it is on the API; is there a better place to bring up API issues such as this?
I tend to disagree because an interface doesn't allow API evolvement that's why you are seeing so many I*Listener2, I*Listener3. We are trying as much as possible to avoid interfaces (I think didn't added any one in 3.3 with the new API) because of these problems.
If it is for the purpose of API evolvement then I would expect to see methods with empty implementations instead of being abstract. It is true that new methods on ColumnViewerEditorActivationListener could be added with empty implementations in order to not break existing subclasses, but if you are going to go down that road then why not give the first methods on the abstract class empty implementations too?
That's a fair point indeed so it would be the adapter implementations already? The current implementation is in fact an interface but comes over to you as a class. We don't currently have any intentions to add API-methods there but you never know. Does your request now turn into having the abstract methods getting no-ops and only the class declared as abstract?
Agree Tom's comment about Interfaces, boy what a mistake that was. Although Martin has a good point, at least the current state provides us a future non-breaking migration path. Martin, your return comments appreciated since there doesn't appear to be interest in changing it to be an interface.
(In reply to comment #0) > The ColumnViewerEditorActivationListener is a fully abstract class, which > forces implementors into extending the activation listener class. This seems > unnecessary in Java when an interface would perform the same job whilst leaving > implementors free to use a more natural class hierarchy. I would expect listeners to be implemented as (anonymous) inner classes. Why would you want to form a class hierarchy? > I realise that this can't easily be fixed now that it is on the API; is there a > better place to bring up API issues such as this? After a release, we cannot change API unless it is binary compatible. API issues can only be brought up during a release cycle. You are more than welcome to look at our milestone builds and provide feedback. That's why we produce milestone builds every six weeks. I'll close this as wontfix but feel free to reopen if you like to see us put empty implementations in ColumnViewerEditorActivationListener.
Now that I understand the reason for the current situation, I have no problem with it staying an abstract class. I do think that giving the methods empty implementations when this approach is taken (at least for listeners) is a sensible step forward though as I think it will lead to an extensible API that takes some of the burden away from implementers. If there are other, similar listener classes then it would be great if you could make the change to these too as consistency across an API is always helpful for developers. Reopening as per comment 5 as I think this change is one that can't make life any harder for implementers, and will probably be better in most cases. I also think it will help with consistency because if/when new methods are added to such an abstract class in the future then you are going to have a mix of abstract methods and empty methods[1]. Changed the summary from "ColumnViewerEditorActivationListener should be an interface" to "Abstract listener classes should have empty methods instead of abstract methods" [1] Unless you change the abstract methods to have an empty implementation at this point, but why not do it sooner rather than later?
I tend to agree with you all 3. Although I think Matin made a really important point in our API evolvement that when we are creating Abstract-Implementations of Listeners (and only for Listeners why we are not automatically providing no-ops instead of abstract implementations)?
(In reply to comment #7) > why we are not automatically providing > no-ops instead of abstract implementations)? It depends on the example. For ColumnViewerEditorActivationListener, because we don't expect clients to implement all four methods, it would be good to provide empty method bodies, but keep the class abstract (since instantiating it does not make sense). You can also follow the pattern of generic listeners, one example of which is SWT's Listener interface. In this case, the extensibility is in the event class, not the listener, which therefore can be an interface.
Boris: I am undecided on whether a general-purpose listener such as the SWT Listener interface is better than listeners written for a specific task; I think both approaches have their pros and cons. If we discuss that here too then this will quickly get out of hand... All: I have been thinking about API evolution, and I don't see how using abstract classes instead of interfaces helps for non-listener classes. If the methods had non-void returns, how would the extension strategy work? For the sake of argument say that you were going to add a class similar to IActionBars now. In the new/current approach you would add a pure abstract class. Is this correct? Now imagine that in the next release you wanted to add a method with a return type, in the same way that the getCoolBarManager() method was added in IActionBars2. How would you do this? The only way I can see would be to add a non-abstract method which threw an UnsupportedOperationException, or worse, returned null. This doesn't strike me as something that makes the system better, you just shift the problem from being a compile-time problem to a runtime problem. With this said, on listener-style classes such as ColumnViewerEditorActivationListener I think we are all agreed that empty methods are superior to abstract methods.
(In reply to comment #9) > If the methods had non-void returns, how would the extension strategy work? It depends - oftentimes, you can return a default value. For example, IWorkbenchPart2 adds getPartName() and getContentDescription(), for which sensible default implementations would be 'return getTitle();' and 'return "";'. As for IActionBars2, I don't understand why the new methods were not added to IActionBars, which is clearly specified as not being intended to be implemented by clients. Have a look at the material linked from http://wiki.eclipse.org/API_Central - if you want to discuss more, I would suggest that we continue the discussion on one of the newsgroups.
Marking as [Viewers] to get this out of inbox and since its where the bug was first reported. However, its a good discussion and much broader.
Mass update - removing 3.4 target. This was one of the bugs I marked for investigation (and potential fixing) in 3.4 but I ran out of time. Please ping on the bug if fixing it would be really important for 3.4, and does not require API changes or feature work.
Hitesh is now responsible for watching bugs in the [Viewers] component area.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.
