Skip to content

Use String.isEmpty() with null guards#2755

Merged
hazendaz merged 2 commits intospotbugs:masterfrom
rovarga:use-isEmpty
Dec 8, 2023
Merged

Use String.isEmpty() with null guards#2755
hazendaz merged 2 commits intospotbugs:masterfrom
rovarga:use-isEmpty

Conversation

@rovarga
Copy link
Copy Markdown
Contributor

@rovarga rovarga commented Dec 7, 2023

There are a few places which check for null-or-empty String, where
the empty part is performed with "".equals().

Rather than loading an explicit literal, use isEmpty() on the string
itself, as we know it is not null.

Signed-off-by: Robert Varga robert.varga@pantheon.tech

Before opening a 'pull request'

  • Search existing issues and pull requests to see if the issue was already discussed.
  • Check our discussions to see if the issue was already discussed.
  • Check for specific project we support to raise the issue on, under spotbugs
  • Do not open intellij plugin issues here, open them at intellij-plugin *

Make sure these boxes are checked before submitting your PR -- thank you!

  • Added an entry into CHANGELOG.md if you have changed SpotBugs code

There are a few places which check for null-or-empty String, where
the empty part is performed with "".equals().

Rather than loading an explicit literal, use isEmpty() on the string
itself, as we know it is not null.

Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
Sum up the change to use .isEmpty() after a null check.

Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
Copy link
Copy Markdown
Contributor

@ThrawnCA ThrawnCA left a comment

Choose a reason for hiding this comment

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

This is fine, but I'm pretty sure an equals check is fairly efficient when the object it's called on is an empty string.

We could also consider using Apache Commons StringUtils.isBlank for this.

@hazendaz hazendaz self-assigned this Dec 8, 2023
@hazendaz
Copy link
Copy Markdown
Member

hazendaz commented Dec 8, 2023

@ThrawnCA Actually this is faster than using string utils. And this is faster than the check it had. See internals of isEmpty() below. Its just checking the length. Other tools are already flagging the same code (believe sonar at least).

public boolean isEmpty() {    
        return value.length == 0;    
    }   

@hazendaz hazendaz merged commit 194f19b into spotbugs:master Dec 8, 2023
@hazendaz
Copy link
Copy Markdown
Member

hazendaz commented Dec 8, 2023

@rovarga Thanks!

@rovarga rovarga deleted the use-isEmpty branch December 10, 2023 20:39
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.

3 participants