Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 478844

Summary:	Inconsistency between class and method definitions for Node
Product:	[Tools] GEF	Reporter:	Greg Watson <g.watson>
Component:	GEF Graph	Assignee:	Alexander Nyßen <nyssen>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	P3	CC:	ed, nyssen
Version:	0.2.0
Target Milestone:	4.0.0 / 3.11.0 (Neon) M3
Hardware:	All
OS:	All
Whiteboard:

Description Greg Watson

2015-10-01 13:59:56 EDT

Node, Edge (and Graph) are declared final, but some methods on Node return Set<? extends ...>. Since the classes are final, it's not possible to extend them.

Unless there's a really good reason for making them final, I'd suggest removing this from the classes. I'd like to be able to extend them to make getting/setting attributes easer. This would also make the model useful for other purposes than just visualizing it.

In any case, the class and methods should be consistent.

Comment 1 Ed Willink

2015-10-01 17:28:17 EDT

Surely a return extends is nothing to do with class extension?. It allows a future implementation of the class to return its internal Set<BetterEdge> directly.

Comment 2 Greg Watson

2015-10-01 17:58:01 EDT

How can you have "class BetterEdge extends Edge" if Edge is final?

Comment 3 Ed Willink

2015-10-01 22:59:42 EDT

You can't. I assumed that it was just the return types and host classes that were final.

Comment 4 Alexander Nyßen

2015-10-02 02:08:17 EDT

The current API is definitely inconsistent. IMHO it does not make sense that Node and Edge are final. 

Nevertheless, the Set<? extends ...> still does not make sense to me as a return type. If we would change to use a 'BetterEdge' subclass internally, I still see no need to return a Set<? extends Edge>. A 'BetterEdge' would still be an Edge.

It could make sense to have Set<? extends Edge> as a method parameter, if Node and Edge aren't final, because an own edge implementation could then be passed in. It could also make sense to have something like Set<E> as a return type where E is a generic class parameter that maybe defined as <E extends Edge>. That would allow to use the complete Graph structure with an own Edge implementation.

Comment 5 Alexander Nyßen

2015-10-02 03:23:31 EDT

For now, I have implemented the following changes:

- Removed final modifier from Node and Edge.
- Ensured Graph constructor can take Set<? extends ...> node and edge sets.
- Ensured all method return types do not use uper bound wildcards.
- Increased version of graph bundles/features to 0.3.0 and adjusted dependencies in other bundles/features.

Is that sufficient for your use case?

Comment 6 Alexander Nyßen

2015-10-20 07:42:46 EDT

Resolving as fixed in 3.11.0 M3. If the solution is not sufficient yet, please reopen.