My function seems to be editing multiple objects

Question

In my Sudoku Android application I have a solve function that solves a Sudoku puzzle (a CellField object). However for some reason when I clone a CellField object and I call the solve method on the cloned object, the solve method solves both of the CellField objects but I only want it to solve the cloned CellField object and not the original object. Any suggestions? Thanks

Here I clone the CellField object (the clone is called temp) and also call the solve method

CellField temp = null;
try {
   temp = board.cf.clone();
} catch (CloneNotSupportedException e) {
   e.printStackTrace();
}
   int x = randInt(0,8);
   int y = randInt(0,8);
while (!temp.getCell(y,x).isEditable && board.cf.getCell(y,x).getValue() == 0) {
   x = randInt(0,8);
   y = randInt(0,8);
}
SudokuSolver solver = new SudokuSolver();
solver.solve(temp);

Here is my solve method and SudokuSolver class

package com.example.czhou.myapplication2;

import java.util.*;

public class SudokuSolver {

    static boolean retry;

    public static int randInt(ArrayList<Integer> candidates) {
        int min = 0;
        int max = candidates.size() - 1;

        //inclusive
        Random rand = new Random();


        int randomNum = rand.nextInt((max - min) + 1) + min;
        int result = candidates.get(randomNum);
        candidates.remove(randomNum);

        return result;

    }

    public boolean solve(CellField field) {
        // write your code here



        boolean isValid = true;

        Set<Integer> toRemove = new HashSet<>();
        int i;
        int j;
        for (int k = 0; k < 9; k++) {
            for (int l = 0; l < 9; l++) {
                field.getCell(k, l).restAlt();
                if (field.getCell(k, l).alt.indexOf(field.getCell(k, l).getValue()) != -1) {
                    field.getCell(k, l).alt.remove(field.getCell(k, l).alt.indexOf(field.getCell(k, l).getValue()));
                }
            }
        }

        for (i = 0; i < 9; i++) {
            for (j = 0; j < 9; j++) {

                if (field.getCell(i,j).getValue() == 0 && field.getCell(i,j).alt.size() == 0){
                    field.getCell(i,j).restAlt();
                }
                if (field.getCell(i, j).isEditable) {
                    toRemove.clear();
                    for (int k = 0; k < 9; k++) {
                        toRemove.add(field.getCell(k, j).getValue());
                    }
                    toRemove.addAll(field.getSectorCandidates(i, j));
                    for (int k = 0; k < 9; k++) {
                        toRemove.add(field.getCell(i, k).getValue());
                    }

                    toRemove.removeAll(Collections.singleton(0));


                    field.getCell(i, j).alt.removeAll(toRemove);
                    if (toRemove.size() == 9 || field.getCell(i, j).alt.size() == 0) {
                        //When there no candidates are available
                        //in the current cell, come here

                        //toRemove.clear();
                        Cell cell;
                        boolean stop = false;
                        backtrack:

                        for (int k = j; !stop; k--) {
                            if (k == -1) {
                                if (i != 0) {
                                    --i;
                                } else {
                                    break;
                                }
                                k = 8;
                            }
                            j = k;
                            // Scan for previous cells have alternative candidates

                            cell = field.getCell(i, k);
                            if (cell.alt.size() > 0 && cell.isEditable) {
                                //bookmark the original cell
                                //int nextCell = k+1;

                                // If found a cell set value as first alternative
                                cell.setValue(cell.alt.get(0));

                                break backtrack;


                            } else if (cell.isEditable){
                                // if no alternatives set cell to 0 and continue backwards
                                cell.setValue(0);
                            }
                        }

                    } else {
                        field.getCell(i, j).setValue(randInt(field.getCell(i, j).alt));
                    }


                }
            }
        }

//        for (int m = 0; m < 9; m++) {
//            for (int l = 0; l < 9; l++) {
//                if (l == 0) {
//                    System.out.println();
//                }
//                System.out.print(field.getCell(m, l).getValue());
//            }
//        }
//        System.out.println();
//        System.out.println("================");


        return isValid;
    }
}

Here is my CellField class

package com.example.czhou.myapplication2;

import android.util.Log;

import java.io.Serializable;
import java.util.*;
import java.util.regex.Matcher;

public class CellField implements Cloneable{
    protected Cell[][] field = new Cell[9][9];
    public CharSequence timeElapsed = "00:00";


    public CellField() {
        for (int i = 0; i < 9; i++) {
            for (int j = 0; j < 9; j++) {
                field[i][j] = new Cell();
            }
        }
    }

    public CellField(CellField another) {
        List<Cell[]> cellfield = Arrays.asList(another.field);
        this.field = (Cell[][]) cellfield.toArray();
    }

    public CellField clone() throws CloneNotSupportedException {
        return (CellField)super.clone();
    }



}

@ Christopher Zhou: what problem getting with current implementation ? — ρяσѕρєя K, Feb 18 '15 at 04:17
I think you should need to make `deep copy` instead of `shallow copy` of `board` object — ρяσѕρєя K, Feb 18 '15 at 04:19

score 0 · Answer 1 · answered Feb 18 '15 at 04:28

The problem is with you Clone method, as @ρяσѕρєя said, you should do a deep copy. Because right now you are returning the same reference. Try something like this:

public CellField clone() throws CloneNotSupportedException {
    CellField clone = new CellField();
    clone.field = this.field;
    clone.timeElapsed = this.timeElapsed;

    return clone;
}

score 0 · Answer 2 · edited May 23 '17 at 10:24

It is a matter of shallow copy versus deep copy.

class SomeClass implements Cloneable {

    // This is the problematic field. It doesn't get cloned the way you think it is.
    public Integer[] field = new Integer[5];

    public SomeClass clone() throws CloneNotSupportedException {
        return (SomeClass) super.clone();
    }

}

public class HelloWorld {

     public static void main(String []args){

        SomeClass first = new SomeClass();

        SomeClass second = null;
        try {
           second = first.clone();
        } catch (CloneNotSupportedException e) {
           e.printStackTrace();
        }

        System.out.println(first.field);
        System.out.println(second.field);
        // Their addresses in memory are the same
        // Modifying one would modify the other
        // first.field == second.field -> true
     }
}

In the above example, I cloned one instance of the class into another and, yet, they share the same field. Mutating fields of the first instance of the class will directly affect the field in the second instance of the class since they both own a reference to it.

Instead of using Cloneable, you could define a copy constructor and perform the cloning on your own.

More advanced details can be found on How to properly override clone method?

My function seems to be editing multiple objects

2 Answers2