Repository navigation
Sonar S4973 "==" and "!=" should not be used when "equals" is overridden #1051
Description
My company develops software that performs automatic remediation of java code. This software detects defects identified for example by Sonar and automatically applies corrective measures. Maybe you would be interested that we experiment our product on your code? For example, we could submit PRs?
When we try the rule S4973 "==" and "!=" should not be used when "equals" is overridden
https://rules.sonarsource.com/java/RSPEC-1698
Below are the changes made by our software to correct this rule.
biojava-genome/src/main/java/org/biojava/nbio/genome/homology/GFF3FromUniprotBlastHits.java
@@ -140,7 +140,7 @@ public class GFF3FromUniprotBlastHits {
}
proteinIndex = proteinIndex + seq.length();
- if (startIndex != null && endIndex != null && startIndex != endIndex) {
+ if (startIndex != null && endIndex != null && !startIndex.equals(endIndex)) {
CDSSequence cdsSequence = cdsSequenceList.get(i);
String hitLabel = "";
if (transcriptSequence.getStrand() == Strand.POSITIVE) {
biojava-genome/src/main/java/org/biojava/nbio/genome/homology/GFF3FromUniprotBlastHits.java
@@ -140,7 +140,7 @@ public class GFF3FromUniprotBlastHits {
}
proteinIndex = proteinIndex + seq.length();
- if (startIndex != null && endIndex != null && startIndex != endIndex) {
+ if (startIndex != null && endIndex != null && !startIndex.equals(endIndex)) {
CDSSequence cdsSequence = cdsSequenceList.get(i);
String hitLabel = "";
if (transcriptSequence.getStrand() == Strand.POSITIVE) {
biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java
@@ -373,7 +373,7 @@ public class EcodInstallation implements EcodDatabase {
return false;
// Re-download old copies of "latest"
- if(updateFrequency != null && requestedVersion == DEFAULT_VERSION ) {
+ if(updateFrequency != null && requestedVersion.equals(DEFAULT_VERSION) ) {
long mod = f.lastModified();
// Time of last update
Date lastUpdate = new Date();
biojava-structure/src/main/java/org/biojava/nbio/structure/io/BondMaker.java
@@ -215,7 +215,7 @@ public class BondMaker {
for (Atom a2:a2s) {
if (a1.getAltLoc() != null && a2.getAltLoc()!=null &&
a1.getAltLoc()!=' ' && a2.getAltLoc()!=' ' &&
- a1.getAltLoc() != a2.getAltLoc()) {
+ !a1.getAltLoc().equals(a2.getAltLoc())) {
logger.debug("Skipping bond between atoms with differently named alt locs {} (altLoc '{}') -- {} (altLoc '{}')",
a1.toString(), a1.getAltLoc(), a2.toString(), a2.getAltLoc());
continue;
biojava-structure/src/main/java/org/biojava/nbio/structure/symmetry/core/HelixSolver.java
@@ -366,7 +366,7 @@ public class HelixSolver {
int permutation = -1;
double minDistSq = Double.MAX_VALUE;
for (int j = 0; j < centers.size(); j++) {
- if (seqClusterId.get(i) == seqClusterId.get(j)) {
+ if (seqClusterId.get(i) != null && seqClusterId.get(i).equals(seqClusterId.get(j))) {
if (!used[j]) {
double dSq = tCenter.distanceSquared(centers.get(j));
if (dSq < minDistSq && dSq <= rmsdThresholdSq) {
biojava-structure/src/main/java/org/biojava/nbio/structure/symmetry/core/RotationSolver.java
@@ -334,7 +334,7 @@ public class RotationSolver implements QuatSymmetrySolver {
int selfaligned = 0;
for (int i = 0; i < permutation.size(); i++) {
int j = permutation.get(i);
- if ( seqClusterId.get(i) != seqClusterId.get(j)) {
+ if ( seqClusterId.get(i) != null && !seqClusterId.get(i).equals(seqClusterId.get(j))) {
return false;
}
if(i == j ) {
biojava-structure/src/main/java/org/biojava/nbio/structure/symmetry/core/SystematicSolver.java
@@ -176,7 +176,7 @@ public class SystematicSolver implements QuatSymmetrySolver {
List<Integer> seqClusterId = subunits.getClusterIds();
for (int i = 0; i < permutation.size(); i++) {
int j = permutation.get(i);
- if (seqClusterId.get(i) != seqClusterId.get(j)) {
+ if (seqClusterId.get(i) != null && !seqClusterId.get(i).equals(seqClusterId.get(j))) {
return false;
}
}
Activity
aalhossary commentedon Jan 10, 2023
For the first 2:
As long as both
startIndexandendIndexare nonnullobjects ofIntegertype, then using.equals()will not add more performance nor readability to the code.Points for whoever is interested
For the third from the end:
Is there a case where both are
null?For the last 2:
is there a case where
seqClusterId.get(i)isnullandseqClusterId.get(j)is not null?jlerbsc commentedon Jan 10, 2023
You are right because usage of the == and != operators for comparing the values of fully memoized boxed primitive types is permitted. But usage of the == and != operators for comparing the values of boxed primitive types that are not fully memoized is permitted only when the range of values represented is guaranteed to be within the ranges specified by the JLS to be fully memoized. And usage of the == and != operators for comparing the values of boxed primitive types is not allowed in all other cases.
This is a rule that is commonly accepted by the community but in this case, even if your code does not hava issues, it is mainly for a homogenization of the management of equalities.
Obviously if you are not convinced of the usefulness of these modifications, you should not implement them of course. ;-)
josemduarte commentedon Jan 10, 2023
Regarding the general question whether we are open to get PRs with improvements as per standard quality metrics, we are more than welcoming them :)
aalhossary commentedon May 9, 2023
closed by #1060