I am doing my homework right now and have a question about refactoring my code in Java. I am working on a Sudoku right now and I need to check if the 3×3 boxes are valid or not. To do that I create a one dimensional array with all the numbers of the boxes and later I compare the value of them. It is working right now but it really isn’t refactored at all. I would really like to know if there is any way to reduce all this copy paste.
public static boolean validFieldParts() {
int counter = 0;
boolean isValid = false;
int[] copyArray1 = new int[field.length];
int[] copyArray2 = new int[field.length];
int[] copyArray3 = new int[field.length];
int[] copyArray4 = new int[field.length];
int[] copyArray5 = new int[field.length];
int[] copyArray6 = new int[field.length];
int[] copyArray7 = new int[field.length];
int[] copyArray8 = new int[field.length];
int[] copyArray9 = new int[field.length];
// copy the array
// 1 große Feld
for (int i = 0; i < field.length / 3; i++) {
for (int j = 0; j < field[i].length / 3; j++) {
copyArray1[i * 3 + j] = field[i][j];
}
}
// 2 große Feld
for (int i = 0; i < field.length / 3; i++) {
for (int j = 3; j < 6; j++) {
copyArray2[i * 3 + j - 3] = field[i][j];
}
}
// 3 große Feld
for (int i = 0; i < field.length / 3; i++) {
for (int j = 6; j < 9; j++) {
copyArray3[i * 3 + j - 6] = field[i][j];
}
}
// 4 große Feld
for (int i = 3; i < 6; i++) {
for (int j = 0; j < field[i].length / 3; j++) {
copyArray4[(i - 3) * 3 + j] = field[i][j];
}
}
// 5 große Feld
for (int i = 3; i < 6; i++) {
for (int j = 3; j < 6; j++) {
copyArray5[(i - 3) * 3 + j - 3] = field[i][j];
}
}
// 6 große Feld
for (int i = 3; i < 6; i++) {
for (int j = 6; j < 9; j++) {
copyArray6[(i - 3) * 3 + j - 6] = field[i][j];
}
}
// 7 große Feld
for (int i = 6; i < 9; i++) {
for (int j = 0; j < field[i].length / 3; j++) {
copyArray7[(i - 6) * 3 + j] = field[i][j];
}
}
// 8 große Feld
for (int i = 6; i < 9; i++) {
for (int j = 3; j < 6; j++) {
copyArray8[(i - 6) * 3 + j - 3] = field[i][j];
}
}
// 9 große Feld
for (int i = 6; i < 9; i++) {
for (int j = 6; j < 9; j++) {
copyArray9[(i - 6) * 3 + j - 6] = field[i][j];
}
}
Arrays.sort(copyArray1);
Arrays.sort(copyArray2);
Arrays.sort(copyArray3);
Arrays.sort(copyArray4);
Arrays.sort(copyArray5);
Arrays.sort(copyArray6);
Arrays.sort(copyArray7);
Arrays.sort(copyArray8);
Arrays.sort(copyArray9);
for (int i = 1; i < copyArray1.length; i++) {
if (copyArray1[i] == copyArray1[i - 1])
counter++;
else
continue;
}
for (int i = 1; i < copyArray2.length; i++) {
if (copyArray2[i] == copyArray2[i - 1])
counter++;
else
continue;
}
for (int i = 1; i < copyArray3.length; i++) {
if (copyArray3[i] == copyArray3[i - 1])
counter++;
else
continue;
}
for (int i = 1; i < copyArray4.length; i++) {
if (copyArray4[i] == copyArray4[i - 1])
counter++;
else
continue;
}
for (int i = 1; i < copyArray5.length; i++) {
if (copyArray5[i] == copyArray5[i - 1])
counter++;
else
continue;
}
for (int i = 1; i < copyArray6.length; i++) {
if (copyArray6[i] == copyArray6[i - 1])
counter++;
else
continue;
}
for (int i = 1; i < copyArray7.length; i++) {
if (copyArray7[i] == copyArray7[i - 1])
counter++;
else
continue;
}
for (int i = 1; i < copyArray8.length; i++) {
if (copyArray8[i] == copyArray8[i - 1])
counter++;
else
continue;
}
for (int i = 1; i < copyArray9.length; i++) {
if (copyArray9[i] == copyArray9[i - 1])
counter++;
else
continue;
}
if (counter > 0)
isValid = false;
else
isValid = true;
return isValid;
}
Answer
Instead of using 9 different arrays and 9 different loops to represent each section of 9, I would have another nested for loop that iterates over each cluster using the same array.
//Iterate over each 'block'
for (int row = 0; row < 3; row++) {
for (int col = 0; col < 3; col++) {
//Iterate over each cell in the block
for (int i = row*3; i < (row+1)*3; i++) {
for (int j = col*3; j < (col+1)*3; j++) {
copyArray[(i - 3) * 3 + j - 3] = field[i][j];
}
}
//Sort array and do duplication check here - return false if dupe found
}
}
return true
This would cut down on the length of your code, although it may not be more efficient.
Instead of using a counter flag, it would be much faster to instead return false whenever you would have incremented the counter, and to return true at the end. This would prevent unnecessary code from running