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

remove MethodInfo and create ActionParameters, ActionResult (or something like this) #104

Closed
Turini opened this issue Sep 23, 2013 · 9 comments · Fixed by #290
Closed

remove MethodInfo and create ActionParameters, ActionResult (or something like this) #104

Turini opened this issue Sep 23, 2013 · 9 comments · Fixed by #290

Comments

@Turini
Copy link
Member

Turini commented Sep 23, 2013

vraptor-brutauth project need easy access to parameters names, and it would be much more interesting if instead of putting a new method into MethodInfo, create a ActionParameter "component" that besides returning the parameters, already has this behavior (return parameters names).

*** not sure if Action prefix is really intuitive, any ideas?

@ghost ghost assigned Turini Sep 23, 2013
@lucascs
Copy link
Member

lucascs commented Sep 23, 2013

Be aware of performance issues when creating these objects.

we could have an Execution object with Parameters and Result or Outcome, and references to the ControllerMethod being executed.

no need to prefixes, since we have the packages ;)

@garcia-jj
Copy link
Member

A note: MethodInfo.getParameter is not to get parameter, but parameter values. I think that this name must be change avoiding mistakes.

@garcia-jj
Copy link
Member

We already have Parameter that have info about method parameter like name, index, annotations, type.

More updates are comming.

@leocwolter
Copy link
Contributor

Just changing the parameters at MethodInfo to Parameter[] or something like that would be a nice and quick solution. I can open a PR if you guys wants me to.

@Turini
Copy link
Member Author

Turini commented Nov 22, 2013

Your PR will be very appreciated =) I think @garcia-jj is working on a kind of
ValuedParameter on this branch... maybe it could be useful, just take a look.

@leocwolter
Copy link
Contributor

Well, that is my solution hahaha. Will this branch be merged soon?

@garcia-jj
Copy link
Member

Sorry for long delay. I was very busy in last weeks. @leonardowolter, if you want, you can improve your changes, because I think that I wont have time in this week

@Turini
Copy link
Member Author

Turini commented Dec 23, 2013

@leonardowolter, @garcia-jj. How is it going? It would be nice do not break compatibility after the RC1.

@garcia-jj
Copy link
Member

I think that is better to change before release RC1. Can we delay the RC1 for some days to think about this change?

Valued parameters are very important to keep code more simple.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants