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: All type members function #1195

Merged
merged 3 commits into from
Mar 11, 2017

Conversation

pvojtechovsky
Copy link
Collaborator

@pvojtechovsky pvojtechovsky commented Feb 22, 2017

This PR Introduces two new mapping functions:

  • SuperHierarchyFunction - which produces all super classes and super interfaces of input CtType
  • AllTypeMembersFunction - which produces all CtTypeMember of input CtType and all super classes and super interfaces.

The AllTypeMembersFunction is then used, as example, to implement CtTypeImpl#getAllFields` in a nicer and more correct way.

This PR is ready for review.

@monperrus
Copy link
Collaborator

Looks good to me. It's tested by all tests using the refactored methods of CtTypeImpl.

One thing: rename SuperHierarchyFunction to InheritanceHierarchyFunction?

@pvojtechovsky
Copy link
Collaborator Author

pvojtechovsky commented Mar 11, 2017

It's tested

yes, it is. It took some time until it passed all these tests. I am really glad that spoon has that level of tests!

rename SuperHierarchyFunction to InheritanceHierarchyFunction?

I vote for a name, which contains direction of traversing through inheritance hieararchy, because I already need an opposite function with name like SubHierarchyFunction. I am open to any change in these two names, but InheritanceHierarchyFunction does not express Super/Sub so it is not good name from that point of view. The both words Inheritance and Hierarchy are good start ...
A) SuperHierarchyFunction, SubHierarchyFunction
B) SuperInheritanceHierarchyFunction, SubInheritanceHierarchyFunction
C) SuperInheritanceFunction, SubInheritanceFunction
D) SuperInheritanceHierarchyFunction, Full/All/Whole/...InheritanceHierarchyFunction ... because in case of SubInheritanceHiearchyFunction it is actually performance expensive to assure the sub inheritance visiting order.
E) ??

@monperrus
Copy link
Collaborator

I like SuperInheritanceHierarchyFunction, SubInheritanceHierarchyFunction

@pvojtechovsky
Copy link
Collaborator Author

If tests pass, then it is finished from my point of view.

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

Thanks!

@pvojtechovsky pvojtechovsky deleted the AllTypeMembersFunction branch March 11, 2017 17:46
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