2

I have a class (Literal). I need to be able to keep an intact Literal instance in memory throughout my application, and have a copy of it which I can alter. I have used two ways to do this:

  1. class Literal implements Cloneable and override Object.clone() method.
  2. Factory constructor:

    public Literal(Literal lit){
         this = lit;
    }
    

In both cases copying doesn't work. Every change I make to the copy, changes the original. Does anybody have any idea what I'm doing wrong?

JasonMArcher
  • 14,195
  • 22
  • 56
  • 52
nkatz
  • 77
  • 5
  • What you doing is only shallow copying.check this post http://stackoverflow.com/questions/64036/how-do-you-make-a-deep-copy-of-an-object-in-java – Umesh Awasthi Dec 21 '11 at 10:32
  • 3
    Are you sure that you work with Java? I am asking this because the second attempt with a *Factory constructor* (usually, this is called *copy constructor*) will result in a compile error "cannot assign a value to final variable this" – nd. Dec 21 '11 at 10:44

2 Answers2

5

since Literal is not a integral type, variables of type Literal holds a reference to the actual value, so this = lit just copies the reference, so your behavior.

You must copy all the member fields recursively to do a "real" copy.

This article has been linked in the accepted answer to the question linked by Umesh in his comment, and I think it clears the conceptual problems you're hitting with your problem.

akappa
  • 10,220
  • 3
  • 39
  • 56
  • Actually I have and it still doesn't work.My actual constructor is public Literal(Literal lit){ this.functor = lit.getFunctor(); this.arguments = lit.getArguments(); this.isCompound = lit.isCompound(); this.isVarOrConst = lit.isVarOrConst(); this.varOrconstName = lit.getVarOrconstName(); this.arity = lit.getArity(); this.subsumedBy = lit.getSubsumedBy(); this.asString = lit.getLiteralAsString(); } – nkatz Dec 21 '11 at 10:40
  • you must do that **recursively**, since the same effect happens in the case your member fields are non-integral types. – akappa Dec 21 '11 at 10:44
  • Check out the answers of this question: http://stackoverflow.com/questions/1175620/in-java-what-is-a-shallow-copy – akappa Dec 21 '11 at 10:46
  • Οκ Ι did that, calling the constructor recursively from within itself. It works now. Thanks a lot!! – nkatz Dec 21 '11 at 11:52
1

If you are going to use copy-constructor, then you will need to make a deep copy of every mutable member variable.

Say your Literal has member variables like this:

private String[] args;
private String s;

Then your copy-constructor would need to do something like this

public Literal(Literal l) {
  // have to deep copy the array, otherwise both instances are sharing the reference
  this.args = Arrays.copyOf(l.args, l.args.length);

  // safe to just copy the reference, String is immutable
  this.s = l.s;
}
ewan.chalmers
  • 16,145
  • 43
  • 60