Override both public boolean Object.equals(Object other), and public int Object.hashCode(), or override neither. Even if you are inheriting a hashCode() from a parent class, consider implementing hashCode and explicitly delegating to your superclass. Example :

// this is bad
public class Bar {
  public boolean equals(Object o) {
      // do some comparison
  }
}

// and so is this
public class Baz {
  public int hashCode() {
      // return some hash value
  }
}

// this is OK
public class Foo {
  public boolean equals(Object other) {
      // do some comparison
  }
  public int hashCode() {
      // return some hash value
  }
}

This rule is deprecated, use squid:S1206 instead.

Overriding a method just to call the same method from the super class without performing any other actions is useless and misleading.

The following code snippet illustrates this rule:

public void doSomething() {                     // Non-Compliant
  super.doSomething();
}

@Override
public boolean isLegal(Action action) {         // Non-Compliant
  return super.isLegal(action);
}

@Override
public boolean isLegal(Action action) {         // Compliant - not simply forwarding the call
  return super.isLegal(new Action(/* ... */));
}

@Id
@Override
public int getId() {                            // Compliant - there is annotation different from @Override
  return super.getId();
}

Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate. This rule allows to check that all package names match a provided regular expression.

The following code snippet illustrates this rule when the regular expression value is "^[a-z]+(\\.[a-z][a-z0-9]*)*$":

package org.Example; // Non-Compliant

package org.example; // Compliant

This class is an inner class, but does not use its embedded reference to the object which created it.  This reference makes the instances of the class larger, and may keep the reference to the creator object alive longer than necessary.  If possible, the class should be made into a static inner class. Since anonymous inner classes cannot be marked as static, doing this will require refactoring the inner class so that it is a named inner class.

This class is an inner class, but does not use its embedded reference to the object which created it except during construction of the inner object.  This reference makes the instances of the class larger, and may keep the reference to the creator object alive longer than necessary.  If possible, the class should be made into a static inner class. Since the reference to the outer object is required during construction of the inner instance, the inner class will need to be refactored so as to pass a reference to the outer instance to the constructor for the inner class.

A large String constant is duplicated across multiple class files. This is likely because a final field is initialized to a String constant, and the Java language mandates that all references to a final field from other classes be inlined into that classfile. See JDK bug 6447475 for a description of an occurrence of this bug in the JDK and how resolving it reduced the size of the JDK by 1 megabyte.

This method accesses the value of a Map entry, using a key that was retrieved from a keySet iterator. It is more efficient to use an iterator on the entrySet of the map, to avoid the Map.get(key) lookup.

This method or field is or uses a Map or Set of URLs. Since both the equals and hashCode method of URL perform domain name resolution, this can result in a big performance hit. See http://michaelscharf.blogspot.com/2006/11/javaneturlequals-and-hashcode-make.html for more information. Consider using java.net.URI instead.

A boxed primitive is allocated just to call toString(). It is more effective to just use the static form of toString which takes the primitive value. So,

This method allocates an object just to call getClass() on it, in order to retrieve the Class object for it. It is simpler to just access the .class property of the class.

This method uses a static method from java.lang.Math on a constant value. This method's result in this case, can be determined statically, and is faster and sometimes more accurate to just use the constant. Methods detected are:

The method seems to be building a String using concatenation in a loop. In each iteration, the String is converted to a StringBuffer/StringBuilder, appended to, and converted back to a String. This can lead to a cost quadratic in the number of iterations, as the growing string is recopied in each iteration.

Better performance can be obtained by using a StringBuffer (or StringBuilder in Java 1.5) explicitly.

For example:

  // This is bad
  String s = "";
  for (int i = 0; i < field.length; ++i) {
    s = s + field[i];
  }

  // This is better
  StringBuffer buf = new StringBuffer();
  for (int i = 0; i < field.length; ++i) {
    buf.append(field[i]);
  }
  String s = buf.toString();

Creating new instances of java.lang.Boolean wastes memory, since Boolean objects are immutable and there are only two useful values of this type.  Use the Boolean.valueOf() method (or Java 1.5 autoboxing) to create Boolean objects instead.

Using new Double(double) is guaranteed to always result in a new object whereas Double.valueOf(double) allows caching of values to be done by the compiler, class library, or JVM. Using of cached values avoids object allocation and the code will be faster.

Unless the class must be compatible with JVMs predating Java 1.5, use either autoboxing or the valueOf() method when creating instances of Double and Float.

Creating a new java.lang.String object using the no-argument constructor wastes memory because the object so created will be functionally indistinguishable from the empty string constant "".  Java guarantees that identical string constants will be represented by the same String object.  Therefore, you should just use the empty string constant directly.

Using the java.lang.String(String) constructor wastes memory because the object so constructed will be functionally indistinguishable from the String passed as a parameter.  Just use the argument String directly.

Using new Integer(int) is guaranteed to always result in a new object whereas Integer.valueOf(int) allows caching of values to be done by the compiler, class library, or JVM. Using of cached values avoids object allocation and the code will be faster.

Values between -128 and 127 are guaranteed to have corresponding cached instances and using valueOf is approximately 3.5 times faster than using constructor. For values outside the constant range the performance of both styles is the same.

Unless the class must be compatible with JVMs predating Java 1.5, use either autoboxing or the valueOf() method when creating instances of Long, Integer, Short, Character, and Byte.

Calling String.toString() is just a redundant operation. Just use the String.

This method uses the toArray() method of a collection derived class, and passes in a zero-length prototype array argument. It is more efficient to use myCollection.toArray(new Foo[myCollection.size()]) If the array passed in is big enough to store all of the elements of the collection, then it is populated and returned directly. This avoids the need to create a second array (by reflection) to return as the result.

A primitive is boxed, and then immediately unboxed. This probably is due to a manual boxing in a place where an unboxed value is required, thus forcing the compiler to immediately undo the work of the boxing.

A primitive boxed value constructed and then immediately converted into a different primitive type (e.g., new Double(d).intValue()). Just perform direct primitive coercion (e.g., (int) d).

This private method is never called. Although it is possible that the method will be invoked through reflection, it is more likely that the method is never used, and should be removed.

This class is an inner class, but does not use its embedded reference to the object which created it.  This reference makes the instances of the class larger, and may keep the reference to the creator object alive longer than necessary.  If possible, the class should be made static.

The equals and hashCode method of URL perform domain name resolution, this can result in a big performance hit. See http://michaelscharf.blogspot.com/2006/11/javaneturlequals-and-hashcode-make.html for more information. Consider using java.net.URI instead.

This field is never read.  Consider removing it from the class.