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

refactor(role): add CtRole ARGUMENT_TYPE and TYPE_ARGUMENT #1622

Merged
merged 1 commit into from
Oct 21, 2017

Conversation

pvojtechovsky
Copy link
Collaborator

This PR introduces new CtRole.PARAMETER_TYPE and CtRole.TYPE_PARAMETER_TYPE_TYPE

CtRole.PARAMETER has values of type CtParameter and CtCatchVariable
CtRole.PARAMETER_TYPE has values of type CtTypeReference

CtRole.TYPE_PARAMETER has values of type CtTypeParameter
CtRole.TYPE_PARAMETER_TYPE_TYPE has values of type CtTypeReference

@monperrus
Copy link
Collaborator

Why is the change of PARAMETER to PARAMETER_TYPE necessary? Is there a confusion with another role PARAMETER?

The proposed TYPE_PARAMETER_TYPE is good. I propose to rename it to TYPE_ARGUMENT (following the convention parameter (definition)/argument (usage) used in Spoon)

@pvojtechovsky
Copy link
Collaborator Author

Why is the change of PARAMETER to PARAMETER_TYPE necessary?

In #1582 you already suggested this change by this comment:

CtExecutableReference CtRole.PARAMETER -> ||CtExecutableReference CtRole.REF_PARAMETERS

There are two ways:
W1) to use PARAMETER in both CtExecutableReference and CtExecutable. Like this

CtExecutable CtRole.PARAMETER
	ItemType: java.util.List<spoon.reflect.declaration.CtParameter<?>>
	get: getParameters() : java.util.List<spoon.reflect.declaration.CtParameter<?>>
	set: setParameters(java.util.List)
	add: addParameter(spoon.reflect.declaration.CtParameter)
----------------------------------------------------------
CtExecutableReference CtRole.PARAMETER
	ItemType: java.util.List<spoon.reflect.reference.CtTypeReference<?>>
	get: getParameters() : java.util.List<spoon.reflect.reference.CtTypeReference<?>>
	set: setParameters(java.util.List)

W2) to have different roles there

CtExecutable CtRole.PARAMETER
	ItemType: java.util.List<spoon.reflect.declaration.CtParameter<?>>
	get: getParameters() : java.util.List<spoon.reflect.declaration.CtParameter<?>>
	set: setParameters(java.util.List)
	add: addParameter(spoon.reflect.declaration.CtParameter)
----------------------------------------------------------
CtExecutableReference CtRole.PARAMETER_TYPE
	ItemType: java.util.List<spoon.reflect.reference.CtTypeReference<?>>
	get: getParameters() : java.util.List<spoon.reflect.reference.CtTypeReference<?>>
	set: setParameters(java.util.List)

I can imagine that both ways has same advantage, but W2 seems to be more consistent with other roles.
With W2 we are near to contract: each role has same type or one common superType of the attribute (item) value.
The only case where W1 is better is CtPath, where it is better for clients to learn less roles. This problem can be solved by role synonyms: CtRole.ARGUMENT, CtRole.PARAMETER_TYPE and CtRole.PARAMETER might have synonym "parameter". It will be not ambiguous, because there is always only one of them on on each type of element.

@monperrus
Copy link
Collaborator

monperrus commented Oct 21, 2017 via email

@pvojtechovsky pvojtechovsky changed the title refactor(role): replace TYPE_/PARAMETER by TYPE_/PARAMETER_TYPE refactor(role): add CtRole ARGUMENT_TYPE and TYPE_ARGUMENT Oct 21, 2017
@pvojtechovsky
Copy link
Collaborator Author

I am finished. It is ready for merge.

@monperrus monperrus merged commit f2736b7 into INRIA:master Oct 21, 2017
@pvojtechovsky pvojtechovsky deleted the refRole3 branch October 21, 2017 14:28
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