Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use count in ParamsRequestCondition#getValueMatchCount #32088

Closed
wants to merge 1 commit into from

Conversation

Ryan-Dia
Copy link
Contributor

The getValueMatchCount method was refactored to use a Stream instead of a for-each loop, eliminating the need for the unnecessary local variable count. This change was made to reduce the risk of accidental modifications during future maintenance, as the implementation is possible without using a local variable. Additionally, the refactoring was done for enhanced stability and maintainability, especially since there was no need to interrupt the loop under specific conditions.

  • The test code was executed and passed successfully.

for loop -> stream
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 23, 2024
@snicoll snicoll added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 23, 2024
@snicoll snicoll added this to the 6.1.4 milestone Jan 23, 2024
@snicoll snicoll self-assigned this Jan 23, 2024
@snicoll snicoll changed the title Refactor getValueMatchCount Use count in ParamsRequestCondition#getValueMatchCount Jan 23, 2024
snicoll added a commit that referenced this pull request Jan 23, 2024
@snicoll snicoll closed this in 484aee0 Jan 23, 2024
@snicoll
Copy link
Member

snicoll commented Jan 23, 2024

@Ryan-Dia congratulations for making your first contribution to Spring Framework.

snicoll added a commit that referenced this pull request Jan 23, 2024
This reverts commit 484aee0, reversing
changes made to 6bd7f02.

See gh-32088
@snicoll snicoll removed this from the 6.1.4 milestone Jan 23, 2024
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed type: task A general task labels Jan 23, 2024
@snicoll
Copy link
Member

snicoll commented Jan 23, 2024

@Ryan-Dia the team has discussed this change today and we've decided to revert it. I had missed that the method you've changed is in a hot code path and we explicitly avoid using the Stream API in such places.

@Ryan-Dia
Copy link
Contributor Author

Thank you for reviewing the changes I proposed. I understand the decision to revert them, recognizing that the modified method is in a hot code path. I appreciate the feedback and will ensure my future contributions are more closely aligned with these best practices

@Ryan-Dia Ryan-Dia deleted the refactor branch January 23, 2024 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants