Rule suggested fix issue
- SonarQube Data Center Edition Version 9.9.3 (build 79811)
- Legacy code incremental improvement on a very strict and restrict corporate setup
- Junit running in Java 21
Hello everyone,
This topic is to submit a suggestion for a new rule, a specific case currently handled by java:S4973 : Strings and Boxed types should be compared using “equals()”. When comparing if a string is empty, the suggestion works, but it could be better.
When the comparison is with empty string == “”
The suggestion fix in the current rule compliant fix:
if (firstName != null && firstName.equals(lastName)) { ... };
My suggestion is to expand the compliant, or create a particular case for empty strings and I suggest something like:
if (!isNull(firstName) && firstName.isEmpty()) { ... };
Hey @jpmartins,
Thank you for your suggestion. In practice, I don’t know how common this pattern is to test for an empty string but it would indeed make the suggested fix easier to understand.
Could you give me a small code sample that reproduces this case. From my end, if I try the following example, I get a sensible, and null-safe, quick-fix applied
void yellIfEmpty(String message) {
if (message == "") {
System.out.println("WHY WOULD YOU GIVE ME AN EMPTY STRING!!!");
}
}
becomes
void yellIfEmpty(String message) {
if ("".equals(message)) {
System.out.println("WHY WOULD YOU GIVE ME AN EMPTY STRING!!!");
}
}
Or are you talking about a comparison where we compare to string references where one always happens to be an empty string? See the following case
static final String EMPTY_STRING = "";
void yellIfEmpty(String message) {
if (message == EMPTY_STRING) {
System.out.println("WHY WOULD YOU GIVE ME AN EMPTY STRING!!!");
}
}
1 Like
Hi, thanks for your reply.
The “”.equals is without a doubt more runtime efficient and results in less code, and a very nice trick, inverting the natural business order of comparison will work, I was not considering, it allows to writing less code for the same result faster, yes.
But the current main aim, for this specific application, is not performance, avoiding not so obvious at first glance code, aiming at code that you can read and understand immediately with minimal risk of doubt or misunderstandings. Almost functional programming style influences.
But your suggestions is perfectly fine and would be an improvement over existing rule one.