Bug 177324 - IterateExpression should attempt to adapt to Collection
Summary: IterateExpression should attempt to adapt to Collection
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.3 M6   Edit
Assignee: JDT-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks: 174633
  Show dependency tree
 
Reported: 2007-03-14 08:57 EDT by Kim Horne CLA
Modified: 2007-03-19 12:16 EDT (History)
6 users (show)

See Also:


Attachments
Patch against core.expressions and core.expressions.tests (19.75 KB, patch)
2007-03-14 15:50 EDT, Kim Horne CLA
no flags Details | Diff
Patch against workbench (5.04 KB, patch)
2007-03-14 15:52 EDT, Kim Horne CLA
no flags Details | Diff
A patch that adapts to Collection (11.37 KB, patch)
2007-03-19 07:42 EDT, Dirk Baeumer CLA
no flags Details | Diff
A patch that introduces new iterfaces to adapt to. (23.49 KB, patch)
2007-03-19 09:16 EDT, Dirk Baeumer CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kim Horne CLA 2007-03-14 08:57:31 EDT
IterateExpression currently fails if the default variable is not a Collection.  It would be very useful if instead of failing it would attempt to adapt the variable to a Collection.  This would allow us to iterate over selections and any number of other useful containers.  Patch forthcoming.
Comment 1 Kim Horne CLA 2007-03-14 08:58:22 EDT
This also applies to CountExpression.
Comment 2 Kim Horne CLA 2007-03-14 15:50:27 EDT
Created attachment 60848 [details]
Patch against core.expressions and core.expressions.tests

Adds the adaptation feature to Iterator and Count via an embedded AdaptExpression element.
Comment 3 Kim Horne CLA 2007-03-14 15:52:58 EDT
Created attachment 60850 [details]
Patch against workbench

Patch that adds the adapter factory to workbench as well as a test case.
Comment 4 Paul Webster CLA 2007-03-16 07:36:18 EDT
This is also important to get in for M6
PW
Comment 5 Dirk Baeumer CLA 2007-03-19 07:41:12 EDT
Hi Kim,

I looked at the patch and I have to say that I am not very happy with the fact that we start adapting certain types to collection in our system to make them consumable by the expression language. A clearer approach would be:

- we introduce a new interface IIterable in core expression which defines the
  necessary interface and the expression language uses that interface to
  adapt to. However this is an API change and needs PMC approval. The approach
  would be more along the lines of Java 5.

Kim, a general question. Why doesn't your code pass in a collection into the evaluation context instead of the selection. This would result in the same result. 
Comment 6 Dirk Baeumer CLA 2007-03-19 07:42:12 EDT
Created attachment 61273 [details]
A patch that adapts to Collection
Comment 7 Paul Webster CLA 2007-03-19 09:07:02 EDT
(In reply to comment #5)
> Kim, a general question. Why doesn't your code pass in a collection into the
> evaluation context instead of the selection. This would result in the same
> result. 

Our evaluation context turns "selection" into a Collection when it places it in the default variable but selection, activeMenuSelection, and activeMenuEditorInput are all ISelections in the evaluation context that are also available.

The user also needs to be able to deal with the ISelection from the 3 variables, hopefully in a reusable way.

PW
Comment 8 Dirk Baeumer CLA 2007-03-19 09:16:43 EDT
Created attachment 61276 [details]
A patch that introduces new iterfaces to adapt to.
Comment 9 Dirk Baeumer CLA 2007-03-19 09:18:05 EDT
The interface are: 

/**
 * Objects that are adaptable to <code>IIterable</code> can be used
 * as the default variable in an iterate expression.
 * 
 * @see IAdaptable
 * @see IAdapterManager
 * 
 * @since 3.3
 */
public interface IIterable {

	/**
	 * Returns an iterator to iterate over the elements.
	 * 
	 * @return an iterator
	 */
	public Iterator iterator();
}

and

/**
 * Objects that are adaptable to <code>ICountable</code> can be used
 * as the default variable in a count expression.
 * 
 * @see IAdaptable
 * @see IAdapterManager
 * 
 * @since 3.3
 */
public interface ICountable {

	/**
	 * Returns the number of elements.
	 * 
	 * @return the number of elements 
	 */
	public int count();
}

Personally I prefer the version with the two new interfaces. Kim, Paul, John what are your thoughts about this ?
Comment 10 John Arthorne CLA 2007-03-19 10:28:49 EDT
Dirk's suggestion looks good to me.   Having specific interfaces for this is much clearer, and avoids having to produce an entire Collection implementation just to obtain either a count or an iterator.  

It's unfortunate to be duplicating the Iterable interace added in Java 5, but core expressions may not be moving to Java 5 for a long time (if ever).
Comment 11 Paul Webster CLA 2007-03-19 10:34:11 EDT
I'm fine with adapting to the 2 new interfaces if we can get PMC approval to add them.

PW
Comment 12 Mike Wilson CLA 2007-03-19 11:54:20 EDT
+1
Comment 13 Dirk Baeumer CLA 2007-03-19 12:16:38 EDT
Fixed as outlines in comment #8 and #9.