Skip to content

Conversation

kwonyonghyun
Copy link
Contributor

In the current codebase, we're using .size() == 0 to check if Lists and Maps are empty. I propose changing this to .isEmpty().

Reasons for the proposed change:

  1. Improved readability: .isEmpty() more clearly expresses the intent of checking whether a collection is empty.
  2. Adherence to Java Collections Framework conventions: isEmpty() is the standard method for checking if a collection is empty.

If there's a specific reason for using size() == 0 instead of isEmpty(), could you please share the rationale?

Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup, @kwonyonghyun! Will you please update any non-Acegi copyright messages to be the current year? When you are done, please squash your commits into one and then go ahead and remove the "Proposal" part of your comment; maybe change it to something like:

Improve readability of empty collection checks

@jzheaux jzheaux changed the title Propose: Replace collection.size() == 0 with isEmpty() for readability Improve readibility of empty collection checks Oct 14, 2024
@kwonyonghyun
Copy link
Contributor Author

Thank you for the review @jzheaux ! Updated non-Acegi copyright messages to be the current year and made it into a single commit like you requested :)

@jzheaux jzheaux self-assigned this Oct 14, 2024
@jzheaux jzheaux added in: core An issue in spring-security-core type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 14, 2024
@jzheaux jzheaux added this to the 6.4.0-RC1 milestone Oct 14, 2024
@jzheaux jzheaux merged commit b8aa788 into spring-projects:main Oct 14, 2024
6 checks passed
@jzheaux
Copy link
Contributor

jzheaux commented Oct 14, 2024

Thanks, @kwonyonghyun! This is now merged into main.

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

Labels

in: core An issue in spring-security-core type: enhancement A general enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants