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:

[source,java,diff-id=1,diff-type=noncompliant]
----
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:

[source,java,diff-id=1,diff-type=compliant]
----
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:

[source,java]
----
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:

[source,java]
----
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

* https://www.artima.com/intv/bloch13.html[Joshua Bloch - Copy Constructor versus Cloning]
* https://docs.oracle.com/javase/8/docs/api/java/lang/Cloneable.html[Interface Cloneable - Java™ Platform, Standard Edition 8 API Specification]
* https://docs.oracle.com/javase/8/docs/api/java/lang/Object.html#clone--[Object.clone - Java™ Platform, Standard Edition 8 API Specification]

=== Related rules

* S2157 - "Cloneables" should implement "clone"
* S1182 - Classes that override "clone" should be "Cloneable" and call "super.clone()"

ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

=== Message

Remove this "clone" implementation; use a copy constructor or copy factory instead.


'''
== Comments And Links
(visible only on this page)

=== relates to: S1182

=== relates to: S2157

=== on 22 May 2015, 19:48:48 Ann Campbell wrote:
consulted \http://stackoverflow.com/questions/2326758/how-to-properly-override-clone-method

=== on 16 Jun 2015, 17:01:47 Nicolas Peru wrote:
Looks good.

endif::env-github,rspecator-view[]
