Improvement to support multiple generics class parameters #992
Conversation
| @@ -90,11 +90,12 @@ private Object instantiateOrAddError(Parameters parameters, List<Message> errors | |||
| private List<Target<Object>> createTargets(ControllerMethod method) { | |||
| Method javaMethod = method.getMethod(); | |||
| List<Target<Object>> targets = new ArrayList<>(); | |||
| TypeExtractor typeExtractor = new TypeExtractor(method.getController().getType()); | |||
There was a problem hiding this comment.
what about changing TypeExtractor to be a CDI managed bean?
so it could be specialized -- extensible
There was a problem hiding this comment.
Ok. Good idea.
What is your suggestion of the package for include this class?
There was a problem hiding this comment.
This is really needed? This method is used in one place only. So I'd keep
as private method in the same class, to using kiss principle.
On Jul 6, 2015 15:39, "Rodrigo Turini" [email protected] wrote:
In
vraptor-core/src/main/java/br/com/caelum/vraptor/http/iogi/IogiParametersProvider.java
#992 (comment):@@ -90,11 +90,12 @@ private Object instantiateOrAddError(Parameters parameters, List errors
private List<Target> createTargets(ControllerMethod method) {
Method javaMethod = method.getMethod();
List<Target> targets = new ArrayList<>();
TypeExtractor typeExtractor = new TypeExtractor(method.getController().getType());what about changing TypeExtractor to be a CDI managed bean?
so it could be specialized -- extensible—
Reply to this email directly or view it on GitHub
https://github.com/caelum/vraptor4/pull/992/files#r33967540.
There was a problem hiding this comment.
In the other hand, extracting to a class are giving us the single responsibility principle.
It's a small class, easily testable, with a well-defined purpose. That's why I'm ok with
this change. But sure, we can keep it as a private method. @francofabio it's your call.
There was a problem hiding this comment.
When I created the TypeExtractor class, my intention was to allow this feature to be reused if there was a need.
Thinking about the opinion of @garcia-jj, it makes sense because VRaptor exists a few years ago and yet there was no need to use this feature.
Maybe have less impact on the project if we keep this feature in their own class (private method).
@Turini, What do you think?
** Sorry for my terrible english. :-/
There was a problem hiding this comment.
That sounds great to me, go ahead @francofabio
|
I didn't test yet, but your PR looks great @francofabio. I've added some comments |
…arametersProvider with generics parameter resolver
|
|
|
Sounds like good, but I want to do some tests in a large app I have before merge. May I? |
|
sure, let us know how the tests went |
|
ok! I'll release |
|
Works fine. I think this changes are ready to merge. |
|
great! thank you again, @francofabio! |
Improvement to support multiple generics class parameters
|
Ok! thanks. |
VRaptor is not able to support multiple generics class parameters.
Currently VRaptor resolves all generics parameter as the first parameter in generic array.
This pull request makes VRaptor able to support multiple generics class parameters.