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

review: feature: Parent function #1211

Merged
merged 1 commit into from Mar 11, 2017

Conversation

pvojtechovsky
Copy link
Collaborator

@pvojtechovsky pvojtechovsky commented Mar 8, 2017

The ParentFunction can now early terminate too.

This PR is ready for review

@monperrus
Copy link
Collaborator

What's the responsibility of ParentFunction?

@pvojtechovsky
Copy link
Collaborator Author

It is a simple function which iterates over all parents, starting from nearest parent and ending on model root package.

@monperrus
Copy link
Collaborator

Hi Pavel,

Could you update testParentFunction to give an example of when and how using ParentFunction#setQuery?

Is there an order to review and merge your remaining PRs?

@pvojtechovsky
Copy link
Collaborator Author

update testParentFunction

ok, but on Saturday. The example is something like
element.map(new ParentFunction(...filter...)).first()

Is there an order to review and merge your remaining PRs?

@pvojtechovsky
Copy link
Collaborator Author

Could you update testParentFunction to give an example of when and how using ParentFunction#setQuery?

There is already test code

assertSame(varStrings.getParent(), varStrings.map(new ParentFunction().includingSelf(false)).first());

which internally uses ParentFunction#setQuery and which profits from optimization made by this PR.

I do not understand, what else should be added.

May be you mean: "Some code which demonstrates how client should use that?" No. Client does not have to be aware of that there is an setQuery method. It is called automatically behind the scene. It should stay hidden for clients. It is a way how to let authors of mapping functions to make them more optimal - to early terminated, if no more results are needed. It is nothing for clients of these functions. So nothing to write into test.

And it is actually impossible to test that ParentFunction really terminated early. It would need a hook (special testing code) in implementation of CtQueryImpl

@monperrus monperrus merged commit f74d235 into INRIA:master Mar 11, 2017
@monperrus
Copy link
Collaborator

Thanks for the explanation and well focused PR.

@pvojtechovsky pvojtechovsky deleted the ParentFunction branch March 11, 2017 15:15
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.

None yet

2 participants