[GitHub] [tomcat] markt-asf commented on issue #242: get a hands on ...

[GitBox <gi...@apache.org>]

markt-asf commented on issue #242: get a hands on ...
URL: https://github.com/apache/tomcat/pull/242#issuecomment-586525169
 
 
   Sorry, this PR cannot be applied. The Servlet API classes cannot have any direct dependencies other than the Java 8 API (which is  why the Tomcat specific hack is implemented the way it is).
   
   It would help to understand the motivation for the refactoring.
   
   There does seem to be an excessive, and unnecessarily verbose, use of one line methods. e.g. `optionsAllowed()`
   
   I'd be interested to see some performance numbers comparing the old and new code. My instinct is the new code will be slower. It probably doesn't matter for doOptions() but I'd still be interested.
   
   Looking at the original code, there is certainly scope for improvement although, again, performance isn't critical here. A simpler refactoring that dropped the booleans, used a StringBuilder, started with an allow header value of `"OPTIONS"` and added to the allow header while looping though the methods is probably as far as I would have gone.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org