Skip to content

Use isEmpty() for String type instead #9494

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

Merged
merged 1 commit into from
Sep 21, 2024
Merged

Use isEmpty() for String type instead #9494

merged 1 commit into from
Sep 21, 2024

Conversation

ngocnhan-tran1996
Copy link
Contributor

No description provided.

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, add you name to the &author list of the affected classes.
And run ./gradlew check locally before pushing.

@ngocnhan-tran1996
Copy link
Contributor Author

I updated

@artembilan
Copy link
Member

I updated

Please, consider in the future to not comment about the changes you have agreed with me to apply.
I'll see new changes in the next commit pushed to the PR.
Feel free to discuss whatever you are not agree.
Also: please, don't squash changes into a single commit: easier for me to review just only new changes and that would preserve a history of PR evolution.
We are squashing on merge anyway.
Extra: see what is our preference for commit message: https://cbea.ms/git-commit/

@artembilan artembilan added this to the 6.4.0-RC1 milestone Sep 21, 2024
@artembilan artembilan merged commit 94d8195 into spring-projects:main Sep 21, 2024
3 checks passed
@artembilan
Copy link
Member

@ngocnhan-tran1996 ,
Thank you for contribution; looking forward for more!

Would you mind sharing with us your experience how have you spotted these minor code smells even in different modules?

@ngocnhan-tran1996
Copy link
Contributor Author

@artembilan

I search keyword length() == 0 in IDE and check each match line in file.
The reason is I also have a task use String#isEmpty instead and it's more easier beacause code smells in files are suggested by sonar

@artembilan
Copy link
Member

@ngocnhan-tran1996 ,

Can you tell us what drives you to do such a search and contribute the fix?
What is an original goal?
We do these changes as well, but not as a dedicated task - rather as side-effect of other task when we touch this or that class and they have smells like you are fixing.
Therefore I'm curious what how have you decided to make this changes since they don't feel very interesting subjects to play with.
Thanks

@ngocnhan-tran1996
Copy link
Contributor Author

@artembilan

I want to be become a better developer, and I think I can improve myself by experience reviewer advice when contribute PR. One more thing, changing old code may be break the present behavior and that must be avoid in my company, includes fix code smells, maybe they don't want deploy a feature just only fix code smells.

Code smells is an very interesting subject to me. It was a better code in the past, but now the techology has changed, it got something smells and I think fixing code smells is also a part that I can apply my knowledges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants