リファクタリングのJavaコードに関する問題

Question

私は今、私の宿題をやっており、自分のコードをJavaでリファクタリングすることについて質問しています。 私は数独に取り組んでおり、3x3ボックスが有効かどうかを確認する必要があります。これを行うために、ボックスのすべての数字を含む1次元配列を作成し、後でそれらの値を比較します。それは今働いていますが、実際にはリファクタリングされていません。私は本当にこのコピー貼り付けをすべて減らす方法があるかどうかを知りたいと思っています。

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 1

は代わり9の各セクションを表すために9つの異なる配列と9つの異なるループを使用して、私は、各クラスタを反復処理は、同じ配列を使用して、そのループの別のネストさを有するであろう。

//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 

これはコードの長さを減らしますが、これは効率的ではありません。

カウンタフラグを使用する代わりに、カウンタをインクリメントしたときにfalseを返し、最後にtrueを返す方がはるかに高速です。これにより不要なコードが実行されなくなります

Answer 2

ここでは完全なリファクタリングです。ここでimprouvements：

• 作成された2つの新しい方法：createCopyと
• isValid削除された未使用の変数counterと1つの2つのサイズの配列を有するisValid
• 置換9つのアレイ。

コードはテストされていませんので、特にcreateCopyメソッドに少し注意してください。

// Creates each block of 9 digits copying digits from field 
// row, col are the block position, starting from upper left 0, 0 to 
// last block 2, 2 
public static int[] createCopy(int[] field, int row, int col) { 
    int[] copy = new int[9]; 
    for (int i = 3 * row; i < 3 * row + 3; i++) { 
     for (int j = 3 * col; j < 3 * col + 3; j++) {    
      copy[(i - 3 * row) * 3 + j - 3 * col] = field[i][j];    
     } 
    } 
    return copy; 
} 

// Check if one block is valid 
private static boolean isValid(int[] copyArray) { 
    Arrays.sort(copyArray); 
    for (int i = 1; i < copyArray.length; i++) { 
     if (copyArray[i] == copyArray[i - 1]) { 
      // Exit immediately if not valid 
      return false; 
     } 
    } 
    return true; 
} 

// Create blocks, then validate them 
// At first not valid block return false 
public static boolean validFieldParts() { 
    int[][] copyArrays = new int[3][3]; 

    for (int row = 0; row < 3; row++) { 
     for (int col = 0; col < 3; col++) { 
      copyArrays[row][col] = createCopy(field, row, col); 
     } 
    } 


    for (int[] copyArray : copyArrays) { 
     if (!isValid(copyArray)) { 
      // Exit immediately if not valid 
      return false; 
     } 
    } 
    return true; 
}