Bugzilla – Full Text Bug Listing
| Summary: | ConnectionEndHandle is declared as a final class. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] GEF | Reporter: | Antoine Toulmé <antoine> | ||||||
| Component: | GEF-Legacy GEF (MVC) | Assignee: | Anthony Hunter <ahunter.eclipse> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | ahunter.eclipse, hudsonr, ppshah | ||||||
| Version: | 3.4 | ||||||||
| Target Milestone: | 3.4.0 (Ganymede) M6 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Linux | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Antoine Toulmé
> It contains a protected method that I want to override in a particular case.
Well, there's only one method in the class...
(In reply to comment #1) > > It contains a protected method that I want to override in a particular case. > > Well, there's only one method in the class... > And three constructors. So what is the minimal number of protected methods necessary to declare the class non-final ? I had to copy the code of the ConnectionEndHandle and ConnectionStartHandle classes into nested private classes, just to change one ls ine in each of them. This code will be committed soon to the STP BPMN component, I will add a link here when it's ready. Unless there's a good reason to mark the class final, we shouldn't. Copying code, however little, has licensing implications. If the class is expected to change, marking it as internal is better than preventing it from being subclassed. (In reply to comment #3) > Unless there's a good reason to mark the class final, we shouldn't. Since ConnectionEndHandle is already public, we can make it non final as requested. (In reply to comment #3) > Unless there's a good reason to mark the class final, we shouldn't. Maybe the class is marked final because it is copied in two places. We have ConnectionStartHandle, and ConnectionEndHandle. As soon as you start subclassing, you need to keep your two duplicate subclasses in sync. Antoine, would a single ConnectionEndpointHandle class that could be either a start or end handle, and is non-final, be preferred? Hi Randy, that would be indeed work. Thanks! Created attachment 85721 [details]
ConnectionEndpointHandle Class
Attaching a proposal for ConnectionEndpointHandle
Could you do the same for ConnectionStartHandle ? I'll be happy to provide a patch, just removing the final keyword if you'd like. Merry Christmas! (In reply to comment #5) > Antoine, would a single ConnectionEndpointHandle class that could be either a > start or end handle, and is non-final, be preferred? The intent was that my patch was a new ConnectionEndpointHandle as Randy described. Now that I look at the code again and the confusion that would be two sets of classes, it is makes the most sense to make ConnectionStartHandle, and ConnectionEndHandle non final. I will make ConnectionStartHandle, and ConnectionEndHandle non final. What about the issue with sub-classing in two different hierarchies? Anything the client overrides has to be copied into both subclasses. Would making both Start and End Handles subclass Endpoint handle work? And then start and end handle could be deprecated. ConnectionEndpointHandle's name would be consistent with the existing ConnectionEndpointLocator, and ConnectionEndpointTracker. Created attachment 86008 [details]
ConnectionEndpointHandle
OK,
Both ConnectionStartHandle and ConnectionEndHandle extend ConnectionEndpointHandle and ConnectionStartHandle and ConnectionEndHandle are deprecated. ConnectionEndpointHandle's name is now consistent with the existing ConnectionEndpointLocator, and ConnectionEndpointTracker.
Missed M5, so moving to M6 Committed the patch to HEAD. Also corrected ConnectionEndpointEditPolicy to use ConnectionEndpointHandle and not the deprecated classes. |