/*
 PointOne type objects has three members.
 Two of type Integer and one of type ThreeD a custom class.
 PointOneClone method takes an argument of PointOne type
 and copy every value from his members to arguments members.
 So the result is a PointOne instance with the exact same values
 but any change we make to it does not affect the original 
 The last four printing methods gives the following results 

 class Copy_Constructor.PointOne
 class Copy_Constructor.PointOne
 true
 false

 So the last result means that i did not succed to make a clone.
 How can i fix it ?

*/

import java.util.logging.Logger;

class ThreeD {

    private Integer z;

    public ThreeD(Integer newz) {
        this.z = newz;
    }

    public int getz() {
        return this.z;
    }

    public void setz(int newz) {
        this.z = newz;
    }

}

class PointOne {

    private ThreeD threed;
    private Integer x;
    private Integer y;

    public PointOne(ThreeD td, Integer x, Integer y) {
        this.x = x;
        this.y = y;
        this.threed = td;
    }

    public PointOne PointOneClone(PointOne point) {

        ThreeD td = new ThreeD(this.threed.getz());
        return new PointOne(td, this.x, this.y);

    }

    public void setxy(int newx, int newy, int newz) {
        this.x = newx;
        this.y = newy;
        this.threed.setz(newz);

    }

    public int getx() {
        return this.x;
    }

    public int gety() {
        return this.y;
    }

    public int getthreed() {
        return this.threed.getz();
    }
}

public class Copy_Constructor_For_Cloning {

    public static void main(String[] args) {
        ThreeD td = new ThreeD(4);
        PointOne p1 = new PointOne(td, 2, 4);
        PointOne p2 = null;
        p2 = p1.PointOneClone(p2);

        System.out.println("clonex :" + p2.getx());
        System.out.println("cloney :" + p2.gety());
        System.out.println("clone threed :" + p2.getthreed());
        System.out.println();

        System.out.println("originalx :" + p1.getx());
        System.out.println("originaly :" + p1.gety());
        System.out.println("original Tthreed :" + p1.getthreed());
        System.out.println();

        System.out.println("Setting new values : 10 , 20 , 30");
        p2.setxy(10, 20, 30);

        System.out.println("clonex new value is :" + p2.getx());
        System.out.println("cloney new value is :" + p2.gety());
        System.out.println("clone threed new value is :" + p2.getthreed());
        System.out.println();

        System.out.println("originalx is still :" + p1.getx());
        System.out.println("originaly is still :" + p1.gety());
        System.out.println("original threed is still :" + p1.getthreed());
        System.out.println();

        System.out.println(p1.getClass());
        System.out.println(p2.getClass());
        System.out.println(p2 != p1);
        System.out.println(p1.equals(p2));


    }
}

Your PointOneClone(PointOne point) method seems weird - you pass it a parameter but you never use that parameter in the method.

The usual convention for a "copy constructor" would be a constructor, not an ordinary method, taking another instance as its only parameter, and initialising its values from that other instance.

It nmay be that yur clone is working, but the equals method (line 113) is the problem. Unless you override it you inherit equals from Object, and the inherited version test for them being exactly the same object.
It looks like you need to override equals for PointOne with a version that returns true if the x, y, and threed.getz() are equal for the two PointOne instances.

Edited 2 Years Ago by JamesCherrill

Right. No need to pass an argument

 public PointOne PointOneClone() {

        ThreeD td = new ThreeD(this.threed.getz());
        return new PointOne(td, this.x, this.y);      

    }


        PointOne p2 = null;
        p2 = p1.PointOneClone();

Even with p1.equals(p2) false
may p2 be considered to be p1 clone ?

That's true, but don't forget that this exercise has a requirements satetment that includes

PointOneClone method takes an argument of PointOne type
and copy every value from his members to arguments members.

so removing the parameter is a reasonable thing to do in general, but not in this case.

Even with p1.equals(p2) false
may p2 be considered to be p1 clone ?

The API for for Object::clone() gives some relevant info on this:

Creates and returns a copy of this object. The precise meaning of "copy" may depend on the class of the object. The general intent is that, for any object x, the expression:

     x.clone() != x

will be true, and that the expression:

     x.clone().getClass() == x.getClass()

will be true, but these are not absolute requirements. While it is typically the case that:

     x.clone().equals(x)

will be true, this is not an absolute requirement. 

You may think that this is pretty useless stuff ("is often X but doesn't have to be" is not much of a spec) and you would be right. That's why the Java clone mechanism is almost never used. The term "clone" is not properly defined.
Most people, faced with a "clone" or "copy" method would assume that it returned a result for which x.clone() != x (ie it's a copy, not just the same object) and x.clone().equals(x) (otherwize what is the meaning of "copy" here?).
x.clone().getClass() == x.getClass() is more complicated than it seems. Eg suppose we have

class A {  
   public A clone() { ...
}
class B extends A { ... }

then what would you expect from

B b = new B();
b.clone().getClass();

... and if you think the answer is "B", how exactly would you implement that?

Edited 2 Years Ago by JamesCherrill

This article has been dead for over six months. Start a new discussion instead.