Copy Constructor useless here?

Question

Consider following situation:

I write a java program for an embedded(modem) device. The SDK doesn't offer Cloneable. Therefore I have in class GsmSignalStrength the method clone(), but it's not from Object.clone(), it is "my" implementation. I would like to know is this useful and correct or should I use a copy constructor in GsmSignalStrength, like in the comment? I don't see the advantage of a copy constructor, because I will have to create another GsmSignalStrength object in my StateMachine.

Here is my code...

File 1 "Status.java":

class Status
{
    Status(GsmSignalStrength gsmSignalStrength)
    {
        initClass(gsmSignalStrength);
    }

    GsmSignalStrength gsmSignalStrength;

    private void initClass(GsmSignalStrength gsmSignalStrength)
    {
        this.gsmSignalStrength = gsmSignalStrength;
    }
}

File 2 "GsmSignalStrength":

class GsmSignalStrength
{
    GsmSignalStrength(byte signalStrength)
    {
        initClass(signalStrength);
    }

    GsmSignalStrength()
    {
        initClass(100);
    }

    byte value;

    public void copyTo(GsmSignalStrength destination)
    {
        destination.value = this.value;
    }

    public GsmSignalStrength clone()
    {
        GsmSignalStrength clonedValue = new GsmSignalStrength();

        this.copyTo(clonedValue);

        return clonedValue;
    }

    /* With copy constructor
        GsmSignalStrength(GsmSignalStrength gsmSignalStrength)
        {
            gsmSignalStrength.value = value;
        }
    */

    private void initClass(byte signalStrength)
    {
        this.value = signalStrength;
    }
}

File 5 "GsmModemHandler":

class GsmModemHandler
{
    GsmModemHandler()
    {
        initClass();
    }

    private GsmSignalStrength m_gsmSignalStrength;

    GsmSignalStrength getGsmSignalStrength()
    {

        ...bla...

        return m_gsmSignalStrength;
    }

    private void initClass()
    {
        m_gsmSignalStrength = new GsmSignalStrength();
    }
}

File 4 "StateMachine":

class StateMachine
{
    StateMachine(GsmModemHandler gsmModemHandler)
    {
        initClass(gsmModemHandler);
    }

    private GsmModemHandler m_gsmModemHandler;

    void doSomething()
    {
        GsmSignalStrength gsmSignalStrength = m_gsmModemHandler.getGsmSignalStrength();
        Status xy = new Status(gsmSignalStrength.clone());

        /* With copy constructor

            GsmSignalStrength gsmSignalStrengthCopy = new GsmSignalStrength(gsmSignalStrength);
            Status xy = new Status(gsmSignalStrengthCopy);
        */
    }

    private void initClass(GsmModemHandler gsmModemHandler)
    {
        m_gsmModemHandler = gsmModemHandler;
    }
}

Answer 1

What you have had in that comment,

GsmSignalStrength(GsmSignalStrength gsmSignalStrength)
    {
        gsmSignalStrength = this;
    }

is not a copy constructor. The line

gsmSignalStrength = this;

has no effect at all because all it does is move a method-local reference to point to this.

A functioning "copy constructor" would be something like

GsmSignalStrength(GsmSignalStrength gsmSignalStrength)
    {
        this.value = gsmSignalStrength.value;
    }

the point being to create an independent object with the same member variables as the original.

Also, unlike in C++, writing

object1 = object2;

does not invoke a copy constructor. All it does is copies the reference -- you end up with two references pointing to the same object. If you want to create a new object you have to invoke the constructor explicitly:

object1 = new GsmSignalStrength( object2 );

---

As of whether it is "useless", that depends on your application. A copy constructor, a clone() method, and your copyTo method all accomplish the same basic task. You can create a new object with the constructor, or with clone(), or create a new object and use copyTo on it -- pick one way to do what you want and stick with it.

Answer 2

I think you should just use clone, but do so carefully. Joshua Bloch's Effective Java states you should use clone judiciously as it's a weak interface and provides no method contract to support the activity. You're also dependent on well-behaved superclass clone behaviour (that is, superclasses use super#clone rather than using a class constructor to create the object (which would create an intance of the wrong class). Note that this is what you've done in your File 2 example - In a non-final class, you'd break any cloning subclasses.

Note that your clone (and each superclass clone) should provide full copies of all ivars. Now beware that you may need to make deep copies of things such as arrays and Lists - that is, create a new instance of the array and add new copies of elements in your own list. This is a "deep copy", as opposed to a "shallow copy". The clone should not share the same data as the original object (though you can probably debate this on a case by case basis).

Also, I don't think there's anything wrong with a copy constructor notion so long as it's well documented what your copy does (e.g. deep versus shallow copy).

Sample code for your example would be:

public class GSMSignalStrength implements Cloneable {
    ...
    public Object clone() throws CloneNotSupportedException {
        GSMSignalStrength result = (GSMSignalStrength)super.clone();
        result.setValue(this.value);
        result.setAnyOldArray(this.deepCopyMyAnyOldArray());
        ...
    }
    ...
}

Hope this helps!

p.s. Not reviewed Cloneable in terms of generics so you may be able to add type-safety around that. In fact, I've never really implemented cloneable at all. In the past (Obj-C world, we did do a lot of cop-constructing in a graphics package and got tied right up with our object graph - when to deep and when to shallow copy). V.complicated.