Cloning a List in Java

Question

I am working on a function in a object that takes in a List of objects as a parameter and clone its contents into its own List. Modifying the new List should not affect the List that was passed in. I know the List would be passed by reference, but would the objects within the list be passed by reference or value? (sorry if this sounds dumb)

I am passing in a list of pieces (pawn, rook etc.) that extends a Piece class. I was thinking of creating a clonePiece() function in the Piece class but I don't know how to go about doing it. This is what I have so far:

    public void copyPieces(List<Piece> whitePieces, List<Piece> blackPieces){
    for (int i = 0; i < whitePieces.size(); i++){
        this.whitePieces.add(whitePieces.get(i).clonePiece());
    }
    for (int i = 0; i < blackPieces.size(); i++){
        this.whitePieces.add(blackPieces.get(i).clonePiece());
    }

How would you go about implementing a clonePiece() function in an abstract class that creates a new instance of its inherited classes?

edit:

public abstract class Piece {
private int color;
private int x;
private int y;

public Piece (int color, int x, int y){
    this.color = color;
    this.y = y;
    this.x = x;
}

public int getColor(){
    return this.color;
}
public int getX(){
    return this.x;
}
public int getY(){
    return this.y;
}

public void move(int x, int y, Board board){
    board.getGameTiles()[this.x][this.y].setToUnoccupied();
    this.x = x;
    this.y = y;
}

public abstract ArrayList<Move> getMoves(Board board);

public Piece clonePiece(){
    return this;
}

}

public class Rook extends Piece{

int x, y, color;
private ArrayList<Move> moves;

public Rook(int color, int x, int y) {
    super(color, x, y);
    this.x = x;
    this.y = y;
    this.color = color;
    moves = new ArrayList<>();
}

@Override
public ArrayList<Move> getMoves(Board board) {

    //moves right
    int a = 1;
    while(UtilFunctions.isInBoundaries(x+a, y)){
        if(!board.getGameTiles()[x+a][y].isTileOccupied()){
            //add move type 0 for passive move
            moves.add(new Move(x, y, x+a, y, 0));
        }
        else{
            if(board.getGameTiles()[x+a][y].getPiece().getColor() != this.color){
                //add move type 1 for attack move
                moves.add(new Move(x, y, x+a, y, 1));
            }
            break;
        }
        a++;
    }

    //moves left
    a = -1;
    while(UtilFunctions.isInBoundaries(x+a, y)){
        if(!board.getGameTiles()[x+a][y].isTileOccupied()){
            //add move type 0 for passive move
            moves.add(new Move(x, y, x+a, y, 0));
        }
        else{
            if(board.getGameTiles()[x+a][y].getPiece().getColor() != this.color){
                //add move type 1 for attack move
                moves.add(new Move(x, y, x+a, y, 1));
            }
            break;
        }
        a++;
    }

    //moves up
    a = 1;
    while(UtilFunctions.isInBoundaries(x, y+a)){
        if(!board.getGameTiles()[x][y+a].isTileOccupied()){
            //add move type 0 for passive move
            moves.add(new Move(x, y, x, y+a, 0));
        }
        else{
            if(board.getGameTiles()[x][y+a].getPiece().getColor() != this.color){
                //add move type 1 for attack move
                moves.add(new Move(x, y, x, y+a, 1));
            }
            break;
        }
        a++;
    }

    //moves down
    a = -1;
    while(UtilFunctions.isInBoundaries(x, y+a)){
        if(!board.getGameTiles()[x][y+a].isTileOccupied()){
            //add move type 0 for passive move
            moves.add(new Move(x, y, x, y+a, 0));
        }
        else{
            if(board.getGameTiles()[x][y+a].getPiece().getColor() != this.color){
                //add move type 1 for attack move
                moves.add(new Move(x, y, x, y+a, 1));
            }
            break;
        }
        a++;
    }

    return moves;
}

}

Answer 1

Your children classes shouldn't have any fields extra than the parent class Piece. There is nothing inherently different at this level. Remove the other fields from the children classes and give then access to the parent Piece class fields.

abstract class Piece {

    Piece(boolean white, int x, int y){
        //set fields
        ...
    }

    public abstract Piece copyPiece();
}

Then implement the method copyPiece() in all children classes. Then call:

public void copyPieces(List<Piece> whitePieces, List<Piece> blackPieces){
    for (Piece whitePiece : whitePieces){
        this.whitePieces.add(whitePiece.copyPiece());
    }
    for (Piece blackPiece : blackPieces){
        this.blackPieces.add(blackPiece.copyPiece());
    }
}

Edit after posting code:

My original answer doesn't fit due to the code posted. Revised accordingly.

A few things seem odd to me. First, color could be a boolean, white, since there are only two colors. Next, it seems strange that your Rook class cannot have set access to the x and y fields of the parent Piece class.

Your clonePiece class does nothing but return itself, this is not a clone at all. A clone would be: return new Piece(this.color, this.x, this.y);

Next, your move(x, y, board) method isn't correct. First, consider not passing in the board and having the class that uses the move method be the one to manipulate it. If you do not want to go this route, you have to set the new position of the piece on the board, not just remove the old one.

Answer 2

Have piece return its own #copy method (Or use Clonable):

public abstract class Piece {

    public Piece copy() {
        //return copy
    }
}

This can similarly be used in subclasses:

public class Rook extends Piece {

    @Override
    public Rook copy() {
        //return copy
    }
}

Then, when you have a list of Piece objects, you simply call #copy on the objects and replace the references in your list. For example:

List<Piece> pieces = /* some list */;
List<Piece> copy = new ArrayList<>(pieces);
copy.replaceAll(Piece::copy); //replace references with copies

Answer 3

First of all, Clonable is commonly recommended to be avoided and I would therefore advice you to instead implement a copy constructor or your own method for copying.

This is my suggested implementation:

In order to benefit from polymorphism, add an abstract method for making deep copies in Piece

public abstract Piece deepCopy();

and corresponding implementations in the subclasses

@Override
public Rook deepCopy() {
    Rook rook = new Rook(this.getColor(), this.getX(), this.getY());
    for (Move m : this.moves) {
        rock.addMove(new Move(m));
    }
    return rook;
}

color, x and y are all of primitive types which means that, at assignment, the value is copied. However, Rook also contains the field move which is a List of the reference type Move. This means that to be able to make a deep copy, also the Move class needs to contain a copy constructor or method for copying. The former is shown here.

public Move(Move move) {
    this(move.x, move.y);
}

Finally, copy your list of pieces either the Java 7 way:

List<Piece> newList = new ArrayList<>();
for (Piece p : oldList) {
    newList.add(p.deepCopy());
}

or the Java 8 way (as previously suggested by Darshan Mehta):

List<Piece> newList = whitePieces.stream()
    .map(p -> p.deepCopy())
    .collect(Collectors.toList());

Answer 4

EDIT Updated the example to support inheritance

You can make Piece class implement Cloneable interface and override clone method in the impl classes. Piece and Rook classes will look like this:

abstract class Piece implements Cloneable{
    int x;
    int y;

    protected void copy(Piece p){
        p.x = this.x;
        p.y = this.y;
    }
}

class Rook extends Piece{
    int z;

    @Override
    public Object clone(){
        Rook rook = new Rook();
        super.copy(rook);
        rook.z = this.z;
        return rook;
    }
}

Once this is done, you can call this method on any piece object and get the deep cloned instance. Below is Java 8 stream example that deep clones list of pieces:

List<Rook> pieces = new ArrayList<>();

List<Piece> cloned = pieces.stream()
                     .map(p -> (Piece) p.clone())
                     .collect(Collectors.toList());

}