search

iamthechad / grails3-recaptcha

ReCaptcha plugin for Grails 3
Other
3 stars 10 forks source link

Converted RecaptchaService to compile static. Added declared types. #10

Closed KerchumA222 closed 9 years ago

iamthechad commented 9 years ago

What problem is this trying to solve?

KerchumA222 commented 9 years ago

Performance is improved when using CompileStatic and declared types. IDE code completion is much better when types are declared. http://docs.groovy-lang.org/latest/html/gapi/groovy/transform/CompileStatic.html http://stackoverflow.com/questions/2262092/explicit-typing-in-groovy-sometimes-or-never#2266716

I also have a little bit more to add to this PR in a few minutes.

iamthechad commented 9 years ago

I understand what the changes do. Is there a performance problem that needs to be solved? I prefer not to break parity with the Grails 2 version of the plugin too much, so I'd like to avoid change for change's sake.

KerchumA222 commented 9 years ago

Parity with the Grails 2 version of the plugin can be closely maintained. CompileStatic has been available for quite some time. Identifying and used declared types is not a difficult thing to do either.

My initial purpose for this PR was to assign types to the parameters of the methods so that IntelliJ has a better idea (see what I did there) of what to suggest for code completion. Once the types were assigned it was very little work to add the CompileStatic part (which is basically a free performance boost at this point).

iamthechad commented 9 years ago

I realize I'm probably going to start sounding like a jerk here, but we still haven't answered my initial question. What problem do these changes solve?

99% of the people who interact with this module will not be affected by changes that improve code completion. No one has yet mentioned that there are performance issues. Again, this feels like change for change's sake.