This rule raises an issue when a class overrides the Object.clone method instead of resorting to a copy constructor or other copy
mechanisms.
Why is this an issue?
The Object.clone / java.lang.Cloneable mechanism in Java should be considered broken for the following reasons and
should, consequently, not be used:
-
Cloneable is a marker interface without API but with a contract about class behavior that the compiler cannot enforce.
This is a bad practice.
- Classes are instantiated without calling their constructor, so possible preconditions cannot be enforced.
- There are implementation flaws by design when overriding
Object.clone, like type casts or the handling of
CloneNotSupportedException exceptions.
How to fix it
A copy constructor, copy factory or a custom copy function are suitable alternatives to the Object.clone /
java.lang.Cloneable mechanism.
Consider the following example:
class Entity implements Cloneable { // Noncompliant, using `Cloneable`
public int value;
public List<Entity> children; // deep copy wanted
Entity() {
EntityManager.register(this); // invariant
}
@Override
public Entity clone() {
try {
Entity copy = (Entity) super.clone(); // invariant not enforced, because no constructor is caled
copy.children = children.stream().map(Entity::clone).toList();
return copy;
} catch (CloneNotSupportedException e) { // this will not happen due to behavioral contract
throw new AssertionError();
}
}
}
The Cloneable / Object.clone mechanism could easily be replaced by copy constructor:
class Entity { // Compliant
public int value;
public List<Entity> children; // deep copy wanted
Entity() {
EntityManager.register(this); // invariant
}
Entity(Entity template) {
value = template.value;
children = template.children.stream().map(Entity::new).toList();
}
}
Or by a factory method:
class Entity { // Compliant
public int value;
public List<Entity> children; // deep copy wanted
Entity() {
EntityManager.register(this); // invariant
}
public static Entity create(Entity template) {
Entity entity = new Entity();
entity.value = template.value;
entity.children = template.children.stream().map(Entity::new).toList();
return Entity;
}
}
Or by a custom copy function:
class Entity { // Compliant
public int value;
public List<Entity> children; // deep copy wanted
Entity() {
EntityManager.register(this); // invariant
}
public Entity copy() {
Entity entity = new Entity();
entity.value = value;
entity.children = children.stream().map(Entity::new).toList();
return Entity;
}
}
Resources
Documentation
Related rules
- S2157 - "Cloneables" should implement "clone"
- S1182 - Classes that override "clone" should be "Cloneable" and call "super.clone()"
