Eamnea
Eamnea

Reputation: 81

Cloning in Java object that got in attribute the same object

I've read this subject : http://howtodoinjava.com/2012/11/08/a-guide-to-object-cloning-in-java/.

I've done a few test and it works. Now, my problem is cloning an object A that got a list of others objects A. For example :

        public class Cell {
            Cell[] listOfCells;
        }

I tried the following code in the class Cell :

        public Object clone() throws CloneNotSupportedException {
            Cell cloned = (Cell) super.clone();

            /* Cloning the list.
             * For example, trying to clone the first cell of the list.
             */
            Cell[] clonedList = new Cell[listOfCells.length];
            clonedList[0] = (Cell) listOfCells[0].clone();
        }

Problem is, when calling the method on that list, each cell will call the method again etc. and then, stackoverflow.

Edit : @PaulBoddington Yes, I'm trying to do a deep copy. Yes, listOfCells will contains this (indirectly). To make it short, each cell has some neighbors (which are cells) which I've representated by a list. What I want to achieve is : cloning a cell and by modyfing this clone, it won't affect the original. For example :

    Cell original;
    Cell cloned = original.clone();

    cloned.die();
    cloned.listOfCells[0].die(); // the first neighbor of the clone

    cloned.showState(); // display dead
    cloned.listOfCells[0].showState; // display dead

    original.showState(); // display alive
    original.listOfCells[0].showState(); // the first neighbor of the original, must be alive

Upvotes: 0

Views: 903

Answers (2)

Paul Boddington
Paul Boddington

Reputation: 37655

I would avoid clone. clone is widely believed to be broken (See e.g. Joshua Bloch's Effective Java, or search this site). Instead you could write your own method.

I've written a method deepCopy that I think works. It uses an IdentityHashMap to spot when a previously encountered Cell is discovered to avoid computing its copy more than once.

Note that this method is still recursive, so you will encounter StackOverflowErrors for very, very large structures anyway, but for objects that can contain themselves directly, or indirectly, there's no way to do it without using something like this.

// My answer assumes there is a constructor like this.
public Cell(int i) {
    listOfCells = new Cell[i];
}

public Cell deepCopy() {
    return deepCopy(this, new IdentityHashMap<Cell, Cell>());
}

private static Cell deepCopy(Cell original, Map<Cell, Cell> map) {
    if (original == null)
        return null;
    Cell copy = map.get(original);
    if (copy != null)
        return copy;
    int length = original.listOfCells.length;
    copy = new Cell(length);
    map.put(original, copy);
    for (int i = 0; i < length; i++)
        copy.listOfCells[i] = deepCopy(original.listOfCells[i], map);
    return copy;
}

Upvotes: 1

AlexWien
AlexWien

Reputation: 28747

In your clone() method replace the line

 clonedList[0] = (Cell) listOfCells[0].clone();

with

for (int i = 0; i < listOfCells.length; i++) {
   Cell clone = this;
   if (listOfCells[i] != this) { // avoid endless loop in case of self reference
      clone = (Cell) listOfCells[i].clone();
   }
   clonedList[i] = clone;
}

Upvotes: 0

Related Questions