Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

design bug: Type of query result? #1085

Closed
pvojtechovsky opened this issue Jan 4, 2017 · 2 comments
Closed

design bug: Type of query result? #1085

pvojtechovsky opened this issue Jan 4, 2017 · 2 comments

Comments

@pvojtechovsky
Copy link
Collaborator

This is design problem of already merged CtQueryable#map and CtQuery#list functions. If we solve it correctly, then we can probably use common interface for CtBaseQuery and CtQuery in PR #1076.

Problem: The query uses this API with method list for returning of the query results:

public interface CtQuery<O> extends CtQueryable {
	/**
	 * actually evaluates the query and returns all the produced elements collected in a List
	 * @return the list of elements collected by the query.
	 */
	List<O> list();
...
}

The type of items of query result list is generic type O.

Problem 1)

  • we cannot assure that query will return only objects of type O. So it can happen that we create List<CtClass<?>>, which will contain for example items of type CtElement ... what can then fail with ClassCastException during iteration through such result list.

Solution:
To use these two list methods instead:

/**
 * actually evaluates the query and returns all the produced elements collected in a List
 * @return the list of elements collected by the query.
 */
List<Object> list();
/**
 * actually evaluates the query and returns only the produced elements collected in a List, 
 * which are assignable to `resultType`
 * @return the filtered list of elements collected by the query.
 */ 
<O> List<O> list(Class<O> resultType);      //may be we do not need such method at all and the first one is enough.

Please note also where the result list type comes from, because it points to another related change request in the design of the queries:

interface CtQueryable {
  <I, R> CtQuery<R> map(CtFunction<I, R> function);
}

Note that type R of return value of the map function defines type of returned CtQuery.
So you can write code like this:

List<CtPackage> p = element.map((CtElement e)->e.getParent(CtPackage.class)).list();

but all this causes compile error:

List<Object> p = element.map((CtElement e)->e.getParent(CtPackage.class)).list();
List<CtElement> p = element.map((CtElement e)->e.getParent(CtPackage.class)).list();

... so I have found that it is sometime quite tricky to make it compilable ...

Another real problem is when you start to create variables to store a query.
This works:

CtQuery<CtPackage> aQuery = element.map((CtElement e)->e.getParent(CtPackage.class));
List<CtPackage> p = aQuery.list();

but this is compilable, but will cause class cast exceptions at runtime

CtQuery<CtPackage> aQuery = element.map((CtElement e)->e.getParent(CtPackage.class));
aQuery.filterChildren(new TypeFilter(CtMethod.class));
List<CtPackage> p = aQuery.list();

... because the returned list will contain instances of CtMethod!!!

I am going to make a small PR which solves only this problem, by using List<Object> CtQuery#list() + by removing generic parameter O from the CtQuery interface.

@monperrus, please give me a feedback if such solution is ok for you. Or do you have different idea?
What about <O> List<O> CtQuery#list(Class<O> filter)?
I will then update PR #1076 too.
I will have probably time for both things tomorrow evening.

@monperrus
Copy link
Collaborator

I am going to make a small PR which solves only this problem, by using List<Object> CtQuery#list() + by removing generic parameter O from the CtQuery interface. @monperrus, please give me a feedback if such solution is ok for you.

OK for me. We have already too many generics in Spoon that are just painful.

What about <O> List<O> CtQuery#list(Class filter)?

It's a good idea.

@pvojtechovsky
Copy link
Collaborator Author

pvojtechovsky commented Jan 5, 2017

note: The solution with List<Object> list() causes such not nice code ... see the type cast of returned list:

public static <E extends CtElement> List<E> getElements(CtElement rootElement, Filter<E> filter) {
	return (List<E>) (List) rootElement.filterChildren(filter).list();
}

and we cannot use #list(Class) because we do not know the class. I guess we can live with that ...
See current PR #1076

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants