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 isEmpty() rather than size()/length() checks. #1590

Merged
merged 1 commit into from Apr 21, 2020

Conversation

jmark99
Copy link
Contributor

@jmark99 jmark99 commented Apr 20, 2020

Per SonarQube recommendations:
Using Collection.size() to test for emptiness works, but using Collection.isEmpty() makes the code more readable and can be more performant. The time complexity of any isEmpty() method implementation should be O(1) whereas some implementations of size() can be O(n).

Per SonarQube recommendations:

Using Collection.size() to test for emptiness works, but using
Collection.isEmpty() makes the code more readable and can be more
performant. The time complexity of any isEmpty() method implementation
should be O(1) whereas some implementations of size() can be O(n).
Copy link
Contributor

@milleruntime milleruntime left a comment

Choose a reason for hiding this comment

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

I checked about half of these changes and they look good. I skimmed the rest to make sure no lines without size checks were changed. Assumed tests pass, looks good.

@ctubbsii ctubbsii added this to In progress in 2.1.0 via automation Apr 20, 2020
@jmark99 jmark99 merged commit 16b10ba into apache:master Apr 21, 2020
2.1.0 automation moved this from In progress to Done Apr 21, 2020
@jmark99 jmark99 deleted the isempty branch April 22, 2020 18:35
jmark99 added a commit to jmark99/accumulo that referenced this pull request Apr 23, 2020
This is a follow-up to apache#1590. After updating code to use isEmpty() this
ticket re-factors to switch order of statements to remove the negation.
Additionally 'if' statement negations were re-factored. A couple of cases
where a statement contained double negations were updated also. The
intent is to increase the clarity of the statements.

This update does not alter the behavior of the code. Hopefully it just
results in more readable code. Since this is primarily a stylistic
update if there is opposition to it then it can be ignored and the
ticket can be closed.
jmark99 added a commit that referenced this pull request Apr 24, 2020
This is a follow-up to #1590. After updating code to use isEmpty() this ticket re-factors to switch order of statements to remove the negation. Additionally 'if' statement negations were re-factored. A couple of cases where a statement contained double negations were updated also. The
intent is to increase the clarity of the statements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
2.1.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants