There was no reason for getRequestUri() to be static, so we removed it.

If the reason the method is static is because it does not access state from the instance, then I think there are several more candidates for static. Based on this, I think that static in getRequestUri() is unnecessary.


And, all the methods in DispatcherServlet used isEmpty(), and only one part used size() > 0. I changed this to isEmpty() for uniformity.

Comment From: sbrannen

Hi @shin-mallang,

There was no reason for getRequestUri() to be static, so we removed it.

If the reason the method is static is because it does not access state from the instance, then I think there are several more candidates for static. Based on this, I think that static in getRequestUri() is unnecessary.

That method is intentionally static because it is an internal helper utility method that should not rely on state in a DispatcherServlet instance. Although it's not a steadfast rule in our code base, we often do that for such use cases, but sometimes we don't think about it. In light of that, I will omit that change when merging this PS and consistently make such utility methods static in a separate commit.

And, all the methods in DispatcherServlet used isEmpty(), and only one part used size() > 0. I changed this to isEmpty() for uniformity.

Good catch. I've repurposed this PR to focus on that and have changed the title accordingly.

Comment From: sbrannen

This has been merged into main in 3932f91117aad8afa082bab63ed7cc44f7c01b70 and revised in 56688ab36130aa9b08c456a2ef90287337c39a74.

Thanks

Comment From: shin-mallang

@sbrannen Thanks for the answer! It's a utility method, so that's why you wrote it with static!

I have one question.

Is there any advantage to doing it in this way other than it reveals the intent of the method is utility?

I'm just curious as to the reasoning behind doing it this way.

Comment From: sbrannen

@sbrannen Thanks for the answer!

You're very welcome.

It's a utility method, so that's why you wrote it with static!

Yes

Is there any advantage to doing it in this way other than it reveals the intent of the method is utility?

The rationale is two-fold:

  1. It reveals the intent.
  2. The use of and invocation of static methods may potentially result in better performance.

Comment From: shin-mallang

@sbrannen

Thank you for your kind answer.🙇🏻‍♂️ It really helped me a lot.