Come si usa il polimorfismo invece di instanceof?

2

Sto cercando di creare un gioco da tavolo astratto. Nel gioco, un giocatore può scegliere di compiere più azioni in un turno, come posizionare, spostare o ruotare un pezzo. Non sono sicuro se la mia implementazione sia negativa o meno. Mi è stato detto dal mio professore che usare instanceof e se affermazioni come questa sono un segno di odore di codice, ma non riesco a pensare a nessun altro modo di implementarlo.

La mia attuale motivazione per implementare il gioco in questo modo è quella di essere in grado di ordinare dinamicamente le diverse azioni in un elenco in modo che la scacchiera possa eseguire le azioni. Tuttavia, ogni azione è diversa e deve essere eseguita in modo diverso. Quindi, dal momento che devo essere in grado di elencare ogni azione in qualsiasi ordine ed eseguire ogni azione, questo era il modo migliore a cui potessi pensare.

    public class Board {

        private Square[][] squares;

        public void execute(List<Action> actions) {
            for (Action action : actions) {
                if (action instanceof MovingAction) {
                    // 
                } else if (action instanceof RotatingAction) {
                    //
                }
            }
        }
    }


    public interface Action {
    }

    public class MovingAction implements Action {

        private Position positionOfPiece;
        private Position targetPosition;

        public Position getPositionOfPiece() {
             return positionOfPiece;
        }

        public Position getTargetPosition() {
             return targetPosition;
        }

        public MovingAction(Position positionOfPiece, Position targetPosition) {
             this.positionOfPiece = positionOfPiece;
             this.targetPosition = targetPosition;
        }

    }

    public class RotationAction implements Action {

         private Position positionOfPiece;
         private Orientation orientation;

         public RotationAction(Position positionOfPiece, Orientation orientation) {
              this.positionOfPiece = positionOfPiece;
              this.orientation = orientation;
         }

         public Position getPositionOfPiece() {
              return positionOfPiece;
         }

         public Orientation getOrientation() {
              return orientation;
         }

    }

Quindi c'è un modo migliore per implementarlo o è questo il modo migliore per farlo?

    
posta Michael Newgate 31.10.2018 - 23:56
fonte

1 risposta

6

I controlli instanceof sono un odore di codice. In sostanza, invece di mantenere tutta la logica all'interno dell'interfaccia e le sue implementazioni, è necessario che il codice chiamante sappia quale tipo di azione è presente anziché semplicemente usarlo. Penso che diventerà un po 'più chiaro man mano che proseguiamo.

Per prima cosa, devi definire i metodi che intendi per le tue azioni da implementare nell'interfaccia. Sarebbe come questo:

public interface Action {
    void execute(Square[][] squares);
}

Definendo il metodo che ti aspetti che il tuo Action esegua, puoi semplicemente chiamarlo. Ora il tuo metodo di elaborazione delle azioni nella scheda è semplificato a questo:

public void execute(List<Action> actions) {
    for (Action action : actions) {
        action.execute(squares);
    }
}

A questo punto, le tue classi Action sono responsabili dell'implementazione di ciò che avevi in instanceof se poi lo controlla e lo mantiene con l'azione che sta facendo il lavoro. Inserirò uno dei seguenti tipi di azione per l'illustrazione:

public class MovingAction implements Action {

    private Position positionOfPiece;
    private Position targetPosition;

    public MovingAction(Position positionOfPiece, Position targetPosition) {
         this.positionOfPiece = positionOfPiece;
         this.targetPosition = targetPosition;
    }

    public void execute(Square[][] squares) {
         Square piece = findSquare(positionOfPiece, squares);
         Square target = findSquare(targetPosition, squares);

         target.setValue(piece.getValue());
         piece.setValue(null);
    }

    private Square findSquare(Position position, Square[][] squares) {
         // ...
    }
}

Questo approccio ora rende più semplice aggiungere nuove azioni senza dover cambiare il modo in cui funziona la scheda. Ogni azione è completamente autonoma.

A questo punto, puoi iniziare a pensare alla logica comune che avresti bisogno di supportare le tue azioni come il metodo findSquare() che ho definito sopra. Questo metodo probabilmente può essere una funzione pura (tutto ciò che è necessario elaborare è passato, e il risultato è lo stesso con gli stessi input). Ma a volte, potresti avere un codice comune che può essere inserito in una classe base. La classe base potrebbe contenere la funzione findSquare() e ora tutte le azioni possono utilizzarla senza ridefinirla in ogni classe.

Il tuo istruttore non ha criticato la funzionalità, ma suggerisce di utilizzare un design che non richiede alla Board di sapere come implementare le azioni. Essenzialmente le tue azioni diventerebbero veramente attive piuttosto che essere semplicemente contenitori di dati passivi. Permette anche di nascondere lo stato interno della tua Azione poiché non ha davvero bisogno di essere esposto.

    
risposta data 01.11.2018 - 01:36
fonte