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

implemented issue #25553(Optimize single character equality checks) #25570

Closed

Conversation

abhishek-abhi
Copy link

No description provided.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 9, 2020
@abhishek-abhi
Copy link
Author

Hi, @snicoll , please review my PR. This time I worked on one raised issue #25553.

@snicoll
Copy link
Member

snicoll commented Aug 10, 2020

@abhishek-abhi thanks for the PR. Please don't mention individual team member, the team already receives a notification when the PR is created.

@sbrannen
Copy link
Member

Thanks for the PR. We are happy to see that you are eager to contribute.

Please note that you actually implemented changes for #25552 and not for #25553.

In addition, both of those issues have descriptions stating "Investigate claims made in Micro optimizations in Java. String.equals() article."

That means that the issues are there as placeholders for the core team to investigate.

If the core team wishes for someone from the community to pick up an issue, we add the status: ideal-for-contribution label.

If you don't see the status: ideal-for-contribution label on an issue, you should post a comment asking the team if a contribution is welcome before submitting a PR.

In addition, some of the changes in this PR are breaking changes that would result in a NullPointerException.

Regarding the tests, if we go to the trouble of updating tests like that we would typically rather refactor the assertions to make use of dedicated AssertJ functionality that better suits the purpose.

In light of the above, I am closing this PR, and the team will investigate #25552 and #25553 in the 5.3 RC1 time frame.

@sbrannen sbrannen closed this Aug 10, 2020
@sbrannen
Copy link
Member

sbrannen commented Aug 10, 2020

For future reference, please make sure that you run a full build locally before submitting any PR.

This PR actually fails the build due to a compilation error:

> Task :spring-core:compileJava FAILED
/source/spring-framework/spring-core/src/main/java/org/springframework/util/StringUtils.java:97: error: cannot find symbol
                return (str == null || str.isEmpty());
                                          ^
  symbol:   method isEmpty()
  location: variable str of type Object
1 error

FAILURE: Build failed with an exception.

If that error is fixed, there are still 3 additional compilation errors:

> Task :spring-messaging:compileJava
/source/spring-framework/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/reactive/AbstractNamedValueMethodArgumentResolver.java:105: error: cannot find symbol
                else if (arg.isEmpty() && namedValueInfo.defaultValue != null) {
                            ^
  symbol:   method isEmpty()
  location: variable arg of type Object
/source/spring-framework/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/AbstractNamedValueMethodArgumentResolver.java:113: error: cannot find symbol
                else if (arg.isEmpty() && namedValueInfo.defaultValue != null) {
                            ^
  symbol:   method isEmpty()
  location: variable arg of type Object
2 errors

> Task :spring-messaging:compileJava FAILED

> Task :spring-web:compileJava
/source/spring-framework/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java:118: error: cannot find symbol
                else if (arg.isEmpty() && namedValueInfo.defaultValue != null) {
                            ^
  symbol:   method isEmpty()
  location: variable arg of type Object
1 error

> Task :spring-web:compileJava FAILED

@abhishek-abhi
Copy link
Author

I will fix these and reopen the PR

@sbrannen
Copy link
Member

I will fix these and reopen the PR

There is no need to do that.

I have added the status: pending-design-work label to both of those issues and have assigned them to myself for the time being.

@abhishek-abhi
Copy link
Author

I will fix these and reopen the PR

There is no need to do that.

I have added the status: pending-design-work label to both of those issues and have assigned them to myself for the time being.

Okay. Can you please assign this to myself if possible. I would love to work on this issue.

@bclozel bclozel removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants