2016-01-19 12 views
5

下記のコードスニペットに基づいています(わかりやすくするために短縮されています)。代入文をif文に入れるのは悪い習慣ですか?

メソッドの目的は、AIが行う最良の動きを決定するために渡されるミニマックスアルゴリズムのリーフノードでのゲームの状態のスコアを決定するために使用されます。

hasThreeInARowAndTwoOpenSpaces_Horizontalは、scoreBoardStateによって呼び出され、何らかの条件が満たされているかどうかを判断するための多くの同様の方法の1つです(行が3つのトークンを持つプレーヤーなど)。それが本当であれば、その条件を満たすプレイヤーの番号を返し、そのプレイヤー(人間のプレイヤーまたはAI)のスコアを上げます。

返される値がゼロでない(スコアを追加する必要があることを意味する)かどうかを確認するには、ifステートメントでメソッドを呼び出す必要があります。 ifステートメント(コードスニペットで行った)内でメソッドから返された値を設定するか、0を返していない場合にメソッドを再度呼び出して変数に設定できます。明らかに2番目の方法はあまり効率的ではありませんが、人間が読めるようになり、起こっていることを気付くのは簡単です。

質問は、ifステートメント内で呼び出されたメソッドによって返される変数を悪い習慣とみなして設定していますか?それとも効率的なので大丈夫ですか?

注:2番目の方法の非効率性はforループ内にあるため、かなり早くなります。この状況は、各条件がテストされるにつれて何度も発生します。ミニマックスアルゴリズム(各ノードは7つのブランチを持つことができます)で各リーフノードに対しても実行されます。深さはわずか3(使用している最小)を意味し、343のリーフノードとブタの深さは7ですを使用して)ほぼ825,000の葉ノードがあります。

/* scores the board state of a root node in a minimax algorithm 
* @gameState a 2 dimensional array that stores values for each space on the 
* board. Stores 0 for empty or 1 or 2 if position is taken by a player 
*/ 
int scoreBoardState (int[][] boardState) { 

    int aiScore = 0; 
    int playerScore = 0; 

    int player = -1; 

    for (int i = 0; i < boardState.length; i++) { 
     for (int j = 0; j < boardState[i].length - 4; j++) { 
      if (j < boardState[i].length - 5 && (player = hasThreeInARowAndTwoOpenSpaces_Horizontal(boardState, i, j)) != 0) { 

       if (player == AI) 
        aiScore += 1000; //magic number entered for clarity 
       else if (player == PLAYER) 
        playerScore += 1000; 

      } 
      else if (i < boardState.length - 4 && j > 2 && (player = hasThreeInARowAndOneOpenSpace_Diagonal_UpperRightToLowerLeft(boardState, i, j)) != 0) { 

       if (player == AI) 
        aiScore += SCORE_THREE_IAR_ONE_OS; 
       else if (player == PL) 
        playerScore += SCORE_THREE_IAR_ONE_OS; 
      } 

     } 

    }  

    return aiScore - playerScore; 
} 

/* 
* checks if, starting from the passed in coordinates, whether there are 3 
* spaces taken by the same player with an empty space on either side in a horizontal direction (left to right). 
* 
* returns the player number if the result is true. returns 0 if the result 
*is false or all spaces are empty 
*/ 
int hasThreeInARowAndTwoOpenSpaces_Horizontal(int[][] boardState, int row, int col) { 
    if (boardState[row][col] == 0 
     && boardState[row][col + 1] == boardState[row][col + 2] && boardState[row][col + 2] == boardState[row][col + 3] 
     && boardState[row][col + 4] == 0) 
    { 
     return boardState[row][col + 1]; 
    } 

    return 0; 
} 
+5

[コードレビュー](https://codereview.stackexchange.com/)に掲載してください。私2¢?あなたの 'if'文はすでに**とても**長いです;代入文を追加しても読みやすくするための正確な記述はありません。[確立されたイディオム](http://www.mkyong.com/java/how-to-read-file-from-java-bufferedreader-example/)がありますが、 'if'で代入するもの。これはそれらの一つではありません。 –

+1

ifステートメントのいずれかを呼び出す前にそれを割り当てるだけでは効率的ではありません。あなたは同じ値を持つ同じ関数への複数の呼び出しを引き続き得ることができます。 – matt

+0

はい、それは悪い習慣です。 Borisが挙げた確立されたイディオムさえ普遍的に受け入れられているわけではない。また、彼が引用した最初の(Java 7より前の)例のリソース管理は間違っています。うまくいけば、静的分析はそのようなコードの著者を叱るでしょう。 – erickson

答えて

5

それは確かにサポートするコードをより困難にコードを読む誰でも、によって予想外であることのリスクを実行します。それはしばしば避けなければならないことです。

どちらの場合も、避けるべきパフォーマンスコストがある場合、ネストされた条件になるように条件を変更できます。だからではなく、この:それはそうでない場合は、論理的に元のコードにあるであろう場合の動作のパフォーマンスペナルティがまだのみ発生している方法

if (j < boardState[i].length - 5) { 
    player = hasThreeInARowAndTwoOpenSpaces_Horizontal(boardState, i, j); 
    if (player != 0) { 

if (j < boardState[i].length - 5 && (player = hasThreeInARowAndTwoOpenSpaces_Horizontal(boardState, i, j)) != 0) { 

あなたはこのような何かがあるかもしれません。しかし、操作の存在とそれに続くローカル変数への代入は、と多くなり、さらに明らかになります。コードを閲覧している人は、何が起こっているのかをすぐに知ることができます。

ここでの利点は、条件自体が非常に明確で簡潔であることです。条件付き比較を長くすると、コードを実行するのが非常に難しくなりますが、単純な比較は簡単です。

ここでの欠点は、ネストされた条件を作成することです。人々はそれらを好まない傾向があります。 (この場合、私の個人的なの意見は、それが2つの悪のほうがはるかに少ないということです)。しかし、それは、それぞれの条件の中の操作を、その可読性が望ましい場合には、適切な名前のメソッドにリファクタリングすることで対処できます。

0

私はそれが悪い習慣であるとは言いません。それが正しく使用されている限り。あなたのケースでは、変数が設定される必要がある前に満たされる必要がある有効な条件があるので、使用法はうまくいきます。それはどちらか一方のオプションでトップの方法では、あなたは、以下を使用して検討するかもしれないが、それは単なる個人的な好みの事である:

condition ? true value : false value 

文は限りが使用されている通り罰金している場合の使用追加のif文が実行されないようにするelse文を使用すると、すべてがうまくいきます。

関連する問題