doDispatch method of Dispatcher servlet class uses hard coded strings, such as "GET" and "HEAD". so, I replaced it with HttpMethod enum.

Comment From: pivotal-issuemaster

@BryceYangS Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

Comment From: pivotal-issuemaster

@BryceYangS Thank you for signing the Contributor License Agreement!

Comment From: TAKETODAY

i think it's not a good idea

Comment From: BryceYangS

i think it's not a good idea

@TAKETODAY Could I hear why you think so? Your feedback will be of great help to my study.

Comment From: dreis2211

I just stumbled over this PR - I hope you don't mind me commenting. I wonder if we should use HttpMethod.GET.name().equals() rather than HttpMethod.GET.matches(). I did a small benchmark (on JDK 11) comparing things, and the former is expectedly a bit faster because it avoids the map access.

MyBenchmark.testMatches                         avgt    6   4,476 ±  1,153   ns/op
MyBenchmark.testNameEquals                      avgt    6   2,462 ±  0,114   ns/op

Given that we deal with DispatcherServlet here, I wonder if we should strive for the better performing option even though it's definitely a micro-optimization.

Comment From: jhoeller

@dreis2211 I suppose we could also fine-tune the HttpMethod.matches implementation to name().equals(method) then - if known to be faster?

Comment From: dreis2211

@jhoeller I was wondering but it actually was like that before - see 3d87718fc6d14a9638682317121f9647e0441cc3. Maybe you remember why that changed?

Comment From: BryceYangS

@jhoeller @dreis2211 Thank all of your comments. I thought the problem about a performance for a while. If there is a reason why HttpMethod.matches was changed, How about using name().equals(method) in DispatcherServlet?

Comment From: TAKETODAY

@BryceYangS Using name().equals(method) is not as good as using the original

Comment From: jhoeller

No idea why I changed that back then... maybe a vague suspicion that hashing within the Map lookup would be faster than a String equality comparison, or maybe just a stylistic notion? In any case, we can change the current HttpMethod.resolve implementation back to a name comparison if that is known to be faster, and then rely on that implementation where appropriate.

Comment From: rstoyanchev

This has been merged and I've also switched HttpMethod matches to String equals comparison with b76e0c482608c664d5d1d701b995dc4ceb0ed340.