Wird eine Zuweisungsanweisung in eine if-Anweisung falsch geschrieben?

Question

Basierend auf den Code-Schnipsel unten (sie wurden zur Verdeutlichung verkürzt).

Der Zweck der Methode scoreBoardState ist es, eine Punktzahl für den Zustand des Spiels an den Blattknoten in einem Minimax-Algorithmus zu bestimmen, der weitergegeben wird, um die beste Bewegung für die KI zu bestimmen.

Die hasThreeInARowAndTwoOpenSpaces_Horizontal ist eine von vielen ähnlichen Methoden, die von scoreBoardState aufgerufen wird, um zu bestimmen, ob eine Bedingung (wie zum Beispiel ein Spieler mit 3 Token in einer Reihe) erfüllt ist. Wenn es wahr ist, gibt es die Nummer des Spielers zurück, der diese Bedingungen erfüllt, und erhöht dann die Punktzahl dieses Spielers (entweder des menschlichen Spielers oder der KI).

Die Methode muss in einer if-Anweisung aufgerufen werden, um zu prüfen, ob der zurückgegebene Wert nicht null ist (was bedeutet, dass ein Score hinzugefügt werden sollte). Ich kann entweder den Wert, der von der Methode zurückgegeben wird, innerhalb der if-Anweisung (was ich im Code-Snippet getan habe) setzen, oder ich kann die Methode erneut aufrufen, wenn sie 0 nicht zurückgibt und sie dann in die Variable setzt. Offensichtlich ist die zweite Methode weniger effizient, aber sie ist menschlicher lesbar und leichter zu bemerken, was passiert.

Die Frage ist, setzt die Variable, die von der in der if-Anweisung aufgerufenen Methode zurückgegeben wird, eine schlechte Methode? Oder ist es in Ordnung, weil es effizienter ist?

Hinweis: Die Ineffizienz der zweiten Methode wächst relativ schnell, da sie innerhalb einer For-Schleife liegt und diese Situation viele Male auftritt, wenn jede Bedingung getestet wird. Es ist auch für jeden Blattknoten in einem Minimax-Algorithmus getan (jeder Knoten kann 7 Zweige haben) bedeutet eine Tiefe von nur 3 (das Minimum, das ich benutze) gibt es 343 Blattknoten undbuta Tiefe von 7 (die höchste bin ich derzeit unter Verwendung) gibt es fast 825.000 Blattknoten.

/* 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; 
} 

Answer 1

Es läuft sicherlich das Risiko von unerwarteten von jemandem zu sein, den Code zu lesen, die den Code schwieriger zu unterstützen macht. Das ist oft eine würdige Sache, die man vermeiden sollte.

In beiden Fällen, wenn Leistungskosten vermieden werden müssen, können Sie die Bedingung ändern, um geschachtelte Bedingungen zu werden. Anstatt also diese:

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

Sie könnte so etwas wie dieses:

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

auf diese Weise die Leistungseinbuße der Operation immer noch nur entstehen, wenn es sonst logisch im ursprünglichen Code wäre. Aber die Existenz der Operation, und ihre anschließende Zuordnung zu einer lokalen Variablen, wird viel mehr offensichtlich. Jeder, der den Code durchsucht, kann mit sehr wenig Nachdenken sofort sehen, was vor sich geht.

Der Vorteil hier ist, dass die Bedingungen selbst sehr klar und prägnant sind. Lange bedingte Vergleiche können Code sehr schwer nachvollziehbar machen, aber ein einfacher Vergleich ist einfach.

Der Nachteil hier ist, dass Sie geschachtelte Bedingungen erstellen. Menschen neigen dazu, diese nicht zu mögen. (Obwohl in diesem Fall meine persönliche Meinung ist, dass es das viel geringere von zwei Übeln ist.) Aber das kann angegangen werden, indem man die Operationen innerhalb jedes bedingten in ihre eigenen passend benannten Methoden umgestaltet, wenn die Lesbarkeit dieses bevorzugt wird.

Answer 2

Ich würde nicht sagen, dass es eine schlechte Übung ist. Solange es richtig verwendet wird. In Ihrem Fall ist die Verwendung in Ordnung, da eine gültige Bedingung erfüllt sein muss, bevor eine Variable festgelegt werden muss. In der Top-Methode, wo sie die eine oder die andere Option ist, könnten Sie die folgende verwenden, aber es ist nur eine persönliche Geschmackssache:

condition ? true value : false value 

Die Verwendung von if-Anweisungen sind in Ordnung, solange sie verwendet werden, Mit else-Anweisungen, um zusätzliche if-Anweisungen zu stoppen, dann ist alles in Ordnung.