This rule raises an issue when a class overrides the Object.clone
method instead of resorting to a copy constructor or other copy
mechanisms.
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. Object.clone
, like type casts or the handling of
CloneNotSupportedException
exceptions. 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; } }