0

I am making some simulations with java. I am trying to simulate, several steps ahead, a node of a tree and then discard all changes and go back to original state. But the clone() does not return me proper resuts.

Node class:

public class TreeNode implements Serializable,Cloneable{

HashMap<Integer, Tracker> trackers;

public Object clone(){
    try{
       TreeNode node = (TreeNode) super.clone();
       HashMap<Integer, Tracker> newTrackers = new  HashMap<Integer, Tracker>;
       for (int i=0:i<4:i++){
          newTrackers.put(i, node.trackers.get(i).clone());
       }
       node.trackers = newTrackers;
       return node;
    }catch(CloneNotSupportException e){
    }
    return null;
}

public run(){

    TreeNode current = root;
    TreeNode CopyNode = (TreeNode) current.clone();
    foo(CopyNode);

    //Here both current and CopyNode have the same changes at trackers                                               
    //made by foo()
}

}

Tracker class:

public class Tracker implements Serializable,Cloneable{
    private final Player player;

public Tracker clone(){
   try{
      Tracker newTracker = (Tracker) super.clone();
      newTracker.player = player.clone();
      return newTracker;
    } catch (CloneNotSupportException e){
    }
   return null;
}

Player Class:

public class Player implements Serializable,Cloneable{
    private int points;

public Player clone(){
   try{
      return (Player) super.clone();
   }catch (CloneNotSupportException e){
   }
       return null;
    }
}

Note: I cannot use apache functions for this like org.apache.commons.lang.SerializationUtils

Emka
  • 340
  • 6
  • 16
  • 1
    You're making a shallow clone, see http://stackoverflow.com/questions/869033/how-do-i-copy-an-object-in-java – tixopi Nov 14 '15 at 22:03
  • 1
    (1) Does not return proper results - what does it return then? (2) this code doesn't compile. (3) **Do not use `clone`**. – RealSkeptic Nov 14 '15 at 22:03
  • Is `SomeOtherClass` immutable or not? If not, you will need to clone/copy those as well. – Paul Boddington Nov 14 '15 at 22:06
  • @RealSkeptic 1) The data results are very extensive, but by doing this I was hoping that the current node will be remain unchanged and all the changes will occur to CopyNode. 2) The code will not compile because it is an abstract version of the problem – Emka Nov 14 '15 at 22:10
  • 1
    When you post code and ask us why it is not working, you should make sure the problem is reproducible. This means that it should compile, and should reproduce the problem you are showing - a [mcve] is the best. And the problem statement should be clear. – RealSkeptic Nov 14 '15 at 22:13
  • @PaulBoddington I updated the question. The problem seems to be that I do not clone this as well, but I could not get it to run, yet. – Emka Nov 14 '15 at 22:43

1 Answers1

2

Change your clone method to:

public TreeNode clone(){
  try {
    TreeNode node = (TreeNode) super.clone();
    node.trackers = (HashMap<Integer, SomeOtherClass>) trackers.clone();
    return node;
  } catch (CloneNotSupportedException e) {
    return null;      
  }
}

Before, both copies of TreeNode had a reference to the same trackers HashMap, hence changing one would change the other. By explicitly making a new copy of trackers however, the two nodes now have two copies of the HashMap, so changing one won't affect the other.

This is assuming that the objects contained in the HashMap aren't being changed. If they are, those changes will be reflected in both copies.

For a more detailed explanation of copying in Java, see the second answer to How do I copy an object in Java?

-- Edit --

If the existing elements in trackers are being modified by foo, you'll need to make copies of each of those too. At the moment you have two copies of a hashmap, but each one has references which are pointing to the same object. Hence editing an object in one hashmap changes that object in the other. You could do something like:

public TreeNode clone(){
  try {
    TreeNode node = (TreeNode) super.clone();
    HashMap<Integer, SomeOtherClass> newTrackers = new HashMap<>();
    for (Integer key : trackers.keySet()) {
      newTrackers.put(key, trackers.get(key).clone());
    }
    node.trackers = newTrackers;
    return node;
  } catch (CloneNotSupportedException e) {
    return null;      
  }
} 

However, this relies on the fact that the objects of SomeOtherClass in trackers themselves have a properly implemented clone method. Otherwise you will end up with the same problem, where any object that they refer to will be the same as the ones in the original hashmap. Unfortunately in Java, there seems to be no easy way of creating a deep clone without explicitly coding it out for all the objects used.

-- Edit 2 --

Change your Tracker clone to:

public Tracker clone() {
  try {
    Tracker newTracker = (Tracker) super.clone();
    newTracker.player = player.clone();
    return newTracker;
  } catch (CloneNotSupportException e){
  }
  return null;
}

Any time an object has references to another object, you'll need to clone those as well. As Player has no references to objects, only primitive types, this should work properly now.

Community
  • 1
  • 1
tixopi
  • 486
  • 3
  • 14
  • I belive that this is the solution, but I cannot get it to run. I updated the question with the changes. Any idea what is wrong? – Emka Nov 14 '15 at 22:42
  • at the run() function the original node and the cloned one has the same values after foo(). I want to change only the node which is passing to foo() – Emka Nov 14 '15 at 22:47
  • Does foo change the objects (not just add/remove entries, but modify the existing elements) that are stored in the HashMap? If so, you'll need to clone the objects in the HasMap as well. I'll update the answer. You should have a read of the second answer to the post I linked to if you haven't done so. – tixopi Nov 14 '15 at 22:51
  • Yes they change both. I have read the other question and tried many things before I post my question. I tryied with copy constructor but it didn't work either. – Emka Nov 14 '15 at 22:55
  • As I said, the Tracker class needs a properly implemented clone. Tracker has a reference to Player which you'll need to copy as well. I'll update answer again – tixopi Nov 14 '15 at 23:51
  • It would definitely be nice if Java had a way to do this kind of thing automatically :) – tixopi Nov 14 '15 at 23:56