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

Parameterized controllers for Java 8 #41

Closed
wants to merge 3 commits into from

Conversation

gitblit
Copy link
Collaborator

@gitblit gitblit commented Dec 19, 2014

Add a controller handler to support named parameter mapping from requests to methods

This handler automatically enjoys whatever type conversion we maintain in StringValue and
will map named request parameters to named method parameters. It requires compiling your
controllers with Java8 and including the parameter metadata. The new module must be compiled with Java8 too.

I don't have an included demo for this.

…ests to methods

This handler automatically enjoys whatever type conversion we maintain in StringValue and
will map named request parameters to named method parameters.  It requires compiling your
controllers with Java8 and including the parameter metadata.
@gitblit
Copy link
Collaborator Author

gitblit commented Dec 19, 2014

Hold off on this. I have another pending PR which I want to rebase this on top of.

@gitblit gitblit closed this Dec 19, 2014
@decebals
Copy link
Member

In my opinion, today the people are abusing annotations. I prefer something more verbose but more clear and without a black magic behind. I don't think that parameterized controller give a real value.
For example I copy this code from ninja framework (and maybe is similar in play framework):

package controllers;

@Singleton
public class AppController {

    public Result userDashboard(
            @PathParam("id") String id, 
            @PathParam("email") String email, 
            @Param("debug") String debug,
            @Params("filters") String [] filters,
            @Header("user-agent") String userAgent) {

        //do something with the parameters...
    }

}

The method userDashboard is too heavy in signature. It's more simple and clear to have a userDashboard without parameters and to write the bellow code in the method body:

int id = request.getParameter("id").toInt();    
String action = request.getParameter("action").toString("new");
Contact contact = request.getEntityFromParameters(Contact.class);

It's nothing magic in the above code. Maybe you write a little more code but it's very clear and I don't need to complicate the pippo core code.

Also I see that this module has java 1.8 as target and for this reason I must compile all the project with java 8, Correct me if I am wrong. Maybe the solution is to create a java-1.8 branch an push everything related to java 8 (lambda for example) in this branch but in this case we will add a new complexity to the project management. If eventually we need this functionality I think that this functionality (strict) can be achieves only using java 1.7.

That's just my two cents.

@gitblit
Copy link
Collaborator Author

gitblit commented Dec 19, 2014

Java7 vs. Java8. If we change the POMS of the non-Java8 modules to spec source & target 7 then you can build using 8 but targeting 7.

On annotation explosion, I agree but there is value in both. In #42 I am proposing optional support for annotated parameters. That doesn't mean you must use it but it's there for those who do want to use it... like me. :)

The nice thing about the Java8 solution is I could rewrite that Ninja example as (I'm going to ignore the header parameter):

package controllers;

@Singleton
public class AppController {

    public Result userDashboard(
            String id, 
            String email, 
            String debug,
            String [] filters {

        //do something with the parameters...
    }

}

or using your example:

void updateContact(int id, Sting action) {
    Contact contact = request.getEntityFromParameters(Contact.class);
}

Both avoid a ton of boilerplate and to my mind that is part of what a good framework should do: offer you the option to work less hard. That is not a requirement, just an option.

Anyway, let's jump over to #42 and depending on the outcome of that I'll rebase this one.

@decebals decebals mentioned this pull request Jun 10, 2015
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