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

feature CtQuery#select(Filter) #1142

Merged
merged 1 commit into from
Jan 21, 2017
Merged

Conversation

pvojtechovsky
Copy link
Collaborator

Actually Filter can be used as mapping function this way:

Filter filter = new SomeFilter();
element.map(new CtFunction<R, Boolean>() {
	@Override
	public Boolean apply(R input) {
		return filter.matches(input);
	}
}).failurePolicy(QueryFailurePolicy.IGNORE);

There are these problems:

  • I needed 2 minutes to write it and 1h to found that failurePolicy must be added too - and as a author I know the queries the best - so current way how legacy filters can be used is ugly and error prone.
  • the failurePolicy is set to whole query, but it should be set only to the query step which uses the filter. Internally there is already a CtQueryImpl#stepFailurePolicy, which should be therefore called instead, but then the code would be even more ugly.

So this PR allows to write:

element.filter(new SomeFilter());

@@ -38,6 +38,11 @@
<R extends CtElement> CtQuery filterChildren(Filter<R> filter);

/**
* @see CtQuery#filter(Filter)
*/
<R extends CtElement> CtQuery filter(Filter<R> filter);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need it in CtQueryable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right. We do not need it. Thanks!

@monperrus
Copy link
Collaborator

you convinced me.

However, I would name the new method select (as in some other languages) to avoid any ambiguity with filterChildren.

Would you add an explanation in filter.md?

@pvojtechovsky pvojtechovsky changed the title feature CtQuery#filter(Filter) feature CtQuery#select(Filter) Jan 21, 2017
@pvojtechovsky
Copy link
Collaborator Author

Done

documentation in filter.md

Yes, I will do it, but I suggest to add it later in another PR. Because

@monperrus monperrus merged commit 2e83cc4 into INRIA:master Jan 21, 2017
@pvojtechovsky pvojtechovsky deleted the mapFilter branch January 21, 2017 08:58
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

Successfully merging this pull request may close these issues.

2 participants