Original object is also changed when values of cloned object are changed.

Question

I am trying to use clone but the original object is also changed when values of cloned object are changed. As you can see KalaGameState does not use any objects so a shallow copy should work.

 /**
     *  This class represents the current state of a Kala game, including 
     *  which player's turn it is along with the state of the board; i.e. the 
      * numbers of stones in each side pit, and each player's 'kala'). 
     */
    public class KalaGameState implements Cloneable
{
    // your code goes here
 private int turn;

    private int[] sidePit;
       private boolean game;


       public Object clone() {
           try {
               return super.clone();
           }
           catch (CloneNotSupportedException e) {
               // This should never happen
               throw new InternalError(e.toString());
           }
       }

    /**
     * Constructs a new GameState with a specified number of stones in each 
     * player's side pits.
     * @param startingStones the number of starting stones in each side pit.
     * @throws InvalidStartingStonesException if startingStones not in the range 1-10.
     */
    public KalaGameState(int startingStones) throws InvalidStartingStonesException
    {
    game=true;
     turn=0;
       sidePit=new int[14];
      for (int i=0; i <= 13 ; i++)
      {
       sidePit[i] = startingStones;

       }
      sidePit[6] =0;
      sidePit[13] =0;
     // your code goes here
    }

    /**
     * Returns the ID of the player whose turn it is.  
     * @return A value of 0 = Player A, 1 = Player B.
     */
    public int getTurn()
    {
       return turn; // your code goes here
    }

    /**
     * Returns the current kala for a specified player.
     * @param playerNum A value of 0 for Player A, 1 for Player B.
     * @throws IllegalPlayerNumException if the playerNum parameter 
     * is not 0 or 1.  
     */     
    public int getKala(int playerNum) throws IllegalPlayerNumException
    {
     if(playerNum!=0 || playerNum!=1)
      throw new IllegalPlayerNumException(playerNum);

     if(playerNum==0)
        return sidePit[6];
     else
      return
      sidePit[13];
     // your code goes here
    }

    /**
     * Returns the current number of stones in the specified pit for 
     * the player whose turn it is.
     * @param sidePitNum the side pit being queried in the range 1-6.
     * @throws IllegalSidePitNumException if the sidePitNum parameter.
     * is not in the range 1-6.
     */ 
    public int getNumStones(int sidePitNum) throws IllegalSidePitNumException
    {
     if(turn==0)
     {
     if(sidePitNum>6 )
      throw new IllegalSidePitNumException(sidePitNum);
     }
     else
      if(sidePitNum>12)
       throw new IllegalSidePitNumException(sidePitNum);


     if(turn==0)
      return sidePit[sidePitNum];
    else
     return sidePit[sidePitNum+6];

     // your code goes here
    }

    /**
     * Returns the current number of stones in the specified pit for a specified player.
     * @param playerNum the player whose kala is sought. (0 = Player A, 1 = Player B).  
     * @param sidePitNum the side pit being queried (in the range 1-6).
     * @throws IllegalPlayerNumException if the playerNum parameter is not 0 or 1.  
     * @throws IllegalSidePitNumException if the sidePitNum parameter is not in the 
     * range 1-6.
     */

    public int getNumStones(int playerNum, int sidePitNum) throws IllegalPlayerNumException, 
                                                                  IllegalSidePitNumException
    {
     /*if(playerNum>2)
      throw new IllegalPlayerNumException(playerNum);
     if(turn==0)
     {
     if(sidePitNum>6 )
      throw new IllegalSidePitNumException(sidePitNum);
     }
     else
      if(sidePitNum>12)
       throw new IllegalSidePitNumException(sidePitNum);
     */

     if(playerNum==0)
       return sidePit[sidePitNum];
     else if(playerNum==1)
      return sidePit[sidePitNum+7];
     else
      return sidePit[sidePitNum];

    }

    /**
     * Returns the current score for a specified player - the player's 
     * kala plus the number of stones in each of their side pits.
     * @param playerNum the player whose kala is sought. (0 = Player A, 1 = Player B).  
     * @throws IllegalPlayerNumException if the playerNum parameter is not 0 or 1.  
     */ 
    public int getScore(int playerNum) throws IllegalPlayerNumException
    {
     if(playerNum>1)
      throw new IllegalPlayerNumException(playerNum);
   int score=0;
     if(playerNum==0)
     {
      for(int i=0;i<=5;i++)
       score=score+sidePit[i];
     score=score+sidePit[6];
     }
     else
     {
      for(int i=7;i<=12;i++)
       score=score+sidePit[i];
     score=score+sidePit[13];
     }

     // your code goes here
     return score;
    }

    private int getSidePitArrayIndex(int sidePitNum) throws IllegalSidePitNumException
    {
     if(turn==0)
     {
     if(sidePitNum>6 )
      throw new IllegalSidePitNumException(sidePitNum);
     }
     else
      if(sidePitNum>12)
       throw new IllegalSidePitNumException(sidePitNum);


       if(turn==0)
       {
     return sidePitNum--;
       }
       else
       {

         return sidePitNum+6;

       }
    }

    public boolean gameOver()
    {
     int stone=0;

     if(turn==0)
     for(int i=0;i<=5;i++)
   stone=stone+getNumStones(i); 

     else
      for(int i=7;i<=12;i++)
       stone=stone+getNumStones(i-7); 

 if (stone==0)
  game=false;

 return game;
    }

    /**
     * Makes a move for the player whose turn it is.
     * @param sidePitNum the side pit being queried (should be in the range 1-6)
     * @throws IllegalSidePitNumException if the sidePitNum parameter is not in the range 1-6.
     * @throws IllegalMoveException if the side pit is empty and has no stones in it.
     */ 
    public void makeMove(int sidePitNum) throws IllegalSidePitNumException, IllegalMoveException
    {
     if(turn==0)
     {
     if(sidePitNum>6 )
      throw new IllegalSidePitNumException(sidePitNum);
     }
     else
      if(sidePitNum>12)
       throw new IllegalSidePitNumException(sidePitNum);
     /*
     if(turn==0)
     {
     if(sidePit[sidePitNum-1]==0)
      throw new IllegalMoveException(sidePitNum);
     }
     else
     { if(sidePit[sidePitNum-1+7]==0)
          throw new IllegalMoveException(sidePitNum);
     } 
      */

      sidePitNum--;
     int temp=sidePitNum;
     int pitNum=sidePitNum+1;
     int stones=getNumStones(turn,sidePitNum);

   if(turn==0)    
     sidePit[sidePitNum]=0;
   else
    {
    sidePitNum=sidePitNum+7;
    sidePit[sidePitNum]=0;
    pitNum=pitNum+7;
    }
  while(stones!=0)
  {
  if(turn==0)
  {
        sidePit[pitNum]=sidePit[pitNum]+1;
    stones--;
    pitNum++;
    if(pitNum==13)
     pitNum=0;
  } 

   else
   {

       sidePit[pitNum]=sidePit[pitNum]+1;
     stones--;
     pitNum++;
     if(pitNum==6)
      pitNum=7;
     else if(pitNum==14)
      pitNum=0;
    }

  }

  boolean res=anotherTurn(pitNum);
  if(!res){
   capture(pitNum,temp);
  if(turn==0)
   turn=1;
  else turn=0;}
   }

    private boolean anotherTurn(int pitNum)
    {pitNum--;
    boolean temp=false;

     if(turn==0)
    {if(pitNum==6)
       {turn=0;
       temp=true;
       }
    }      
      else
       if(pitNum==-1)
        {turn=1;
        temp=true;
        }
          return temp;
    }


    private void capture(int pitNum, int pit)
    {

     pitNum--; 
if(turn==0){
 if(sidePit[pitNum]==1 && pitNum<6)
  {
  if(pitNum==0)
  {
  sidePit[6]=sidePit[6]+sidePit[12]+1;
  sidePit[12]=0;

  }
  else if(pitNum==1)
  {
   sidePit[6]=sidePit[6]+sidePit[11]+1;
   sidePit[11]=0;

   }
  else if(pitNum==2)
  {
   sidePit[6]=sidePit[6]+sidePit[10]+1;
   sidePit[10]=0;
   }
  else if(pitNum==3)
  {
   sidePit[6]=sidePit[6]+sidePit[9]+1;
   sidePit[9]=0;
   }
  else if(pitNum==4)
  {
   sidePit[6]=sidePit[6]+sidePit[8]+1;
   sidePit[8]=0;
   }
  else if(pitNum==5)
  {
   sidePit[6]=sidePit[6]+sidePit[7]+1;
   sidePit[7]=0;
   }
  sidePit[pitNum]=0;
  }
}
  if(turn==1)
  { //pitNum=pitNum;
   if(sidePit[pitNum]==1 && pit+7>6)
    {
    if(pitNum==7)
    {
    sidePit[13]=sidePit[13]+sidePit[5]+1;
    sidePit[7]=0;
    }
    else if(pitNum==8)
    {
     sidePit[13]=sidePit[13]+sidePit[4]+1;
     sidePit[4]=0;
     }
    else if(pitNum==9)
    {
     sidePit[13]=sidePit[13]+sidePit[3]+1;
     sidePit[3]=0;
     }
    else if(pitNum==10)
    {
     sidePit[13]=sidePit[13]+sidePit[2]+1;
     sidePit[2]=0;
     }
    else if(pitNum==11)
    {
     sidePit[13]=sidePit[13]+sidePit[1]+1;
     sidePit[1]=0;
     }
    else if(pitNum==12)
    {
     sidePit[13]=sidePit[13]+sidePit[0]+1;
     sidePit[0]=0;
     }
    sidePit[pitNum]=0;
    }

  }

    }
}



import java.io.BufferedReader;
import java.io.InputStreamReader;


public class RandomPlayer extends KalaPlayer{
 //KalaGameState state; 
 public int chooseMove(KalaGameState gs) throws NoMoveAvailableException
 {int[] moves;
  moves=getMoves(gs);
  try{
  for(int i=0;i<=5;i++)
   System.out.println(moves[i]);

  for(int i=0;i<=5;i++)
  {
   if(moves[i]==1)
   {

     KalaGameState state=(KalaGameState) gs.clone();

    state.makeMove(moves[i]);
     gs.getTurn();
    moves[i]=evalValue(state.getScore(0),state.getScore(1));
       }
  }

  }
  catch(IllegalMoveException e)
  {
   System.out.println(e);
   //chooseMove(state);
  }
  return 10;
 }

 private int evalValue(int score0,int score1)
 {
  int score=0;
  //int score0=0;
 // int score1=0;
  //getScore(0);
  //score1=state.getScore(1);

  //if((state.getTurn())==0)
  score=score1-score0;
  //else
   //score=score1-score0;
  System.out.println("score: "+score);
  return score;
 }
public int[] getMoves(KalaGameState gs)

{
  int[] moves=new int[6];

  for(int i=1;i<=6;i++)
  {
   if(gs.getNumStones(i)!=0)
    moves[i-1]=1;
   else moves[i-1]=0;
  }
return moves;  
}
}

Can you explain what is going wrong, please?

When i clone KalaGameState

KalaGameState state=(KalaGameState) gs.clone();

    state.makeMove(moves[i]);
     gs.getTurn();

when i call state.makemove(...) the changes it makes are also reflected in gs instance

Answer 1

You have obtained what is known as a "shallow copy" of the object. Essentially all of the primitives are copied over, including the object references (the array in this case).

What you want is a "deep copy". There are a couple of ways to go about this, one is to use clone the other s the create a new constructor that takes the object and copies it all (you would have to make a new array and copy each item into it one by one, for example).

The simplest way for you to get started (not necessarily the best in the end) would be to read this article.

Answer 2

You need to clone() the int[] sidePit (which otherwise would get shared between this and its clone().

KalaGameState clone = (KalaGameState) super.clone();
clone.sidePit = this.sidePit.clone();
return clone;

This should fix your problem, but you should also know that cloning in Java is considered broken.

Questions on clone()

• How to properly override clone method?
• Clone() vs Copy constructor- which is recommended in java

Quotes

If you've read the item about cloning in my book, especially if you read between the lines, you will know that I think clone is deeply broken.

Source: Josh Bloch on Design - Copy Constructor versus Cloning

Questions on mutation seen by all shared references

• java immutable object question
  • Sharing of a String[]; String is immutable; arrays aren't

Answer 3

The issue is sidePit is a reference to an array. The call to super.clone() is creating a new KalaGameState, but sidePit is pointing to the same array. The super.clone() approach (and the whole Clonable interface) only works if your object consists of primitives and Strings (or any other immutable objects). However, for your class you need a clone that looks something like this:

public KalaGameState clone() {
    KalaGameState result = new KalaGameState();
    result.sidePit = sidePit.clone();
    return result;
}