Same old question about clone(). How ???

[Yaroslav Chinskiy]

All.

I have a delema , I want to create an immutable message. It is not strictly immutable, I just want to create a copy of a message before sending it.
The code looks similar to this:

    
public abstract class Message<T extends Cloneable> implements Cloneable{
    private final T body;

    protected Message(T body) {
        if(body == null) throw new NullPointerException("Message body is NULL");
        this.body= body;
    }

    public T getBody() {
        return body;
    }

    public Message clone() throws CloneNotSupportedException { 
        Message message = (Message) super.clone();
        message.body = this.body.clone();
        return message;
    }
}

But clone is protected so the code will not compile. Do you have any advice on how to clone properly?

[Campbell Ritchie]

Make it completely immutable?

public T getBody()
{
   return body.clone();
}

Then you don't need to clone it.

Or change the body to a String, which is already immutable, so there is no need for the cloning of the body?

[Rob Spoor]

Campbell Ritchie wrote:Make it completely immutable?

public T getBody()
{
   return body.clone();
}

Then you don't need to clone it.

That just shifts the problem, since body.clone() is still not accessible.

[Steve Luke]

Since you know (as a programmer) that the T implements Cloneable, and therefore has a public clone(), then you could use reflection:

import java.lang.reflect.InvocationTargetException;

public class Message<T extends Cloneable> implements Cloneable
{
    private T body;

    public Message(T b)
    {
        this.body = b;
    }

    public Message<T> clone() throws CloneNotSupportedException
    {
        T bd = null;
        try
        {
            bd = (T) body.getClass()
                            .getDeclaredMethod("clone", (Class<?>) null)
                            .invoke(this.body, (Object[]) null);
        } catch (IllegalArgumentException e)
        {
            e.printStackTrace();
        } catch (SecurityException e)
        {
            e.printStackTrace();
        } catch (IllegalAccessException e)
        {
            e.printStackTrace();
        } catch (InvocationTargetException e)
        {
            e.printStackTrace();
        } catch (NoSuchMethodException e)
        {
            e.printStackTrace();
        }

        if (bd == null)
            throw new CloneNotSupportedException(
                            "The clone() method for class " + body.getClass()
                                            + " could not be called on " + body
                                            + ".");

        Message<T> message = (Message<T>) super.clone();
        message.body = bd;

        return message;
    }

}

Note however, that regardless of the Generic nature or not, the body variable can not be a final field - since you will be unable to assign to the message.body field. If you need the body to be final, you are better off using a Copy Constructor for the Message - rather than clone().

[Campbell Ritchie]

I take your points. A copy constructor would be better.

[Yaroslav Chinskiy]

Steve,

Good catch about final. I will fix it.
I wanted to use reflection as the last reserve. You code will accomplish what I need, but I was hoping for an easier solution. I guess who ever designed clone() did not intend it to be used outside of inheritance. sad.... :banghead:

[Steve Luke]

Hi Yaroslav,

If you wanted to treat your object more like an immutable object, you should probably protect the body a little bit better (for example, copying it on its way into the Message through the constructor, and copying it on its way out of the getBody method) to protect it from being changed by external sources. Example:

import java.lang.reflect.InvocationTargetException;

public class Message<T extends Cloneable> implements Cloneable
{
    private final T body;

    public Message(T b)
    {   //Hold a copy of the passed in body so external source can't change it on us
        this.body = this.cloneOfBodyType(b);
    }

    protected Message(Message<T> m)
    {   //Copy CTOR - sets this body to a clone of the passed in one to keep both safe
        this.body = this.cloneOfBodyType(m.body);
    }
    
    public T getBody()
    {   //Send a copy of body so external source can't change it on us
        return cloneOfBodyType(this.body);
    }

    //With all that copying going on, centralize the means of cloning the body
    private T cloneOfBodyType(T source)
    {
        if (source == null)
            throw new IllegalArgumentException("The source of the message body can not be null.");
        
        T bd = null;

        try
        {
            bd = (T) source.getClass().getDeclaredMethod("clone",
                            (Class<?>) null).invoke(source, (Object[]) null);
        } catch (IllegalArgumentException e)
        {
            e.printStackTrace();
        } catch (SecurityException e)
        {
            e.printStackTrace();
        } catch (IllegalAccessException e)
        {
            e.printStackTrace();
        } catch (InvocationTargetException e)
        {
            e.printStackTrace();
        } catch (NoSuchMethodException e)
        {
            e.printStackTrace();
        }

        if (bd == null)
            throw new IllegalArgumentException("The clone() method for class "
                            + source.getClass() + " could not be called on "
                            + source + ".  The body can not be copied.");
        
        return bd;
    }
    
    //I never combined a clone() method with a Copy CTOR before.  Is it safe?
    public Message<T> clone() throws CloneNotSupportedException
    {
        Message<T> message = new Message<T>(this);
        return message;
    }
}

[Rob Spoor]

Steve Luke wrote:Since you know (as a programmer) that the T implements Cloneable, and therefore has a public clone()

No you don't. The chances are high that clone() will be public, but it's perfectly legal to implement Cloneable and keep clone() protected, or not even overwrite it at all.

Also, the contract for clone() does not require it to return an object of the same class. The returned object can even be unequal to the source. It is in fact very, very loose in what a programmer should do when overriding it. Of course most programmers use super.clone() in the entire chain, but it's not required to do so.

Yaroslav Chinskiy wrote:I guess who ever designed clone() did not intend it to be used outside of inheritance. sad.... :banghead:

A lot of people think the bigger problem is in Cloneable - they think it should enforce clone() to become public. I don't know; yes it would be useful because you could check for Cloneable, cast to it where necessary and simply call clone(). But that does force you to make clone() public, while you may want to keep it protected (don't ask me why).