spring-framework
spring-framework copied to clipboard
Clarify intention of `toString()` usage in log messages in `HandlerMethod`
The formatErrorForReturnValue method in ServletInvocableHandlerMethod previously included an unnecessary call to toString() and a dangling "in" at the end of the formatted string.
Since the surrounding context already provides sufficient information, both the toString() call and the " in" suffix have been removed to improve clarity.
No functional changes.
After revisiting the HandlerMethod implementation, I realized that I misunderstood the original purpose of the toString() call.
Initially, I thought the usage of toString() was redundant or unclear. However, I now understand that it returns the description string, which actually provides useful context like ClassName#methodName(args...). sorry for the confusion.
That said, I'd like to propose a small improvement.
The method name toString() by itself doesn't make the intention very clear to someone unfamiliar with the HandlerMethod class. I believe replacing it with getShortLogMessage() could make the code more readable and self-explanatory, especially for contributors or maintainers not deeply familiar with this class.
The output remains similar, but the method name communicates its purpose more directly.
If this suggestion doesn't align with the direction of the project, please feel free to suggest an alternative approach or close this PR if it's unnecessary.
The method name
toString()by itself doesn't make the intention very clear to someone unfamiliar with the HandlerMethod class. I believe replacing it withgetShortLogMessage()could make the code more readable and self-explanatory, especially for contributors or maintainers not deeply familiar with this class.The output remains similar, but the method name communicates its purpose more directly.
If this suggestion doesn't align with the direction of the project, please feel free to suggest an alternative approach or close this PR if it's unnecessary.
I'm not so sure that it's worth changing things here.
If we were to change something, I suppose we could either make the HandlerMethod.description field protected or introduce a getDescription() method in HandlerMethod.
We'll discuss it within the team and get back to you.
@sbrannen Thank you for your feedback! I’ll be waiting to hear back after your team discussion.
Instead of changing the code or introducing a dedicated method to retrieve the description, I think we should just update the Javadoc for HandlerMethod.toString() to document that it's used in log/error messages and should typically include the method signature of the handler method.
If you would like to repurpose this PR to do that, feel free to do so.
Otherwise, I'll just add the Javadoc myself.
Thanks for the feedback! 🙏 I’ve removed the previous changes and updated this PR to only include the Javadoc for HandlerMethod.toString() as suggested. I’d appreciate it if you could take a quick look and let me know if the updated Javadoc looks good to you.
This has been merged into main.
Thanks