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

Added a method to know whether a given method is part of a CtType #880

Merged
merged 3 commits into from
Oct 5, 2016
Merged

Added a method to know whether a given method is part of a CtType #880

merged 3 commits into from
Oct 5, 2016

Conversation

arnobl
Copy link
Contributor

@arnobl arnobl commented Oct 4, 2016

Hey all,
As suggested on the mailing list, here is a PR to add a method in CtType to know whether a given type has a method (inherited or not, abstract or implemented). Look at the Javadoc for more details. Let me know if you want me to change the behaviour of the added method (replacing PR 870).

@monperrus
Copy link
Collaborator

This method is quite handy to know whether one can call a method on typed variable.

I wonder if using a signature-based test would be better than an object-based test.

Ie replacing method.getParent() == this by

final String sig = method.getSignature();
for(CtMethod<?> m : getMethods()) {
    if(m.getSignature().equals(sig)) {
        return true;
    }
}

Obviously, this behavior supersedes the current one.

What do you think?

@arnobl
Copy link
Contributor Author

arnobl commented Oct 5, 2016

Yes, the object-based test is not mandatory but just an optimisation. I can remove it. Tell me.

@monperrus
Copy link
Collaborator

let's go for a signature-based test then

@@ -216,6 +216,14 @@
List<CtMethod<?>> getMethodsByName(String name);

/**
* Searches in the type for the given method.
* Super classes and implemented interfaces are considered.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The matching criterion is that the signatures are identical.

@monperrus monperrus merged commit 3ff413f into INRIA:master Oct 5, 2016
@monperrus
Copy link
Collaborator

thanks a lot for this contribution

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