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

Add an optional Java 8+ controller handler to use compiled parameter names #43

Closed
wants to merge 1 commit into from

Conversation

gitblit
Copy link
Collaborator

@gitblit gitblit commented Dec 19, 2014

Now that we've merged #42, a Java8+ ControllerHandler becomes basically this:

   @Override
    protected String getParameterName(Method method, int i) {

        String name = super.getParameterName(method, i);

        if (StringUtils.isNullOrEmpty(name)) {
            // parameter is not named via annotation
            // try looking for the parameter name in the compiled .class file
            Parameter parameter = method.getParameters()[i];
            if (parameter.isNamePresent()) {
                name = parameter.getName();
            }
        }

        return name;
    }

I am struggling to name this module and I'm not sure if you will agree to merge an optional Java8 module anyway. Suggestions or feedback welcome.

@gitblit
Copy link
Collaborator Author

gitblit commented Dec 19, 2014

Another alternative is the approach I took in Iciql. Use reflection in Java 7 to execute the Java 8 API, if its there. I can do that and we both win - but that's your call.

https://github.com/gitblit/iciql/blob/master/src/main/java/com/iciql/DaoProxy.java#L455

@decebals
Copy link
Member

What I don't understand is the value of using the new Parameter class from Java 8 without String value() default ""; on Param annotation.

@gitblit
Copy link
Collaborator Author

gitblit commented Dec 30, 2014

I've been delaying my response till I got #53 and it's precursor changes posted.

In another conversation you mentioned being wary of annotation overload. I agree but we need annotations to automate some boilerplate operations.

In my mind for Java 8 the following signature is purely decorative: void something(@Param int id).
It is functionally equivalent to void something(int id).

The only thing you have gained is explicit clarity on the source of the method argument but I think that the controller method arguments can be assumed to be Request parameters unless explicitly annotated otherwise (e.g. @Body and @Form from #53).

Only the controller handler can execute the controller method so it must be able to provide all method arguments. The default handler is limited to providing method arguments from the Request. That implies a direct relationship between the controller method arguments and the Request. I suppose you could argue that a custom controller handler could provide other non-Request objects - but I think that is unlikely because we already provide (primitive) injection solutions.

To summarize, I think that void something(@Param int id) is of limited value but if you feel strongly about it then we can support it. My main concern would be trying to use that technique in Java 7 OR Java 8 without the -parameters flag; Pippo will crash on startup. But I guess that is an easy fix. Your call.

@gitblit
Copy link
Collaborator Author

gitblit commented Jan 7, 2015

This can wait till Pippo switches to J8.

@gitblit gitblit closed this Jan 7, 2015
@gitblit gitblit deleted the java8_controllerhandler branch January 9, 2015 22:17
@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.

None yet

2 participants