Eliminazione di `instanceof`

Question

In un gioco basato su sprite che sto scrivendo, ogni campo in una griglia 2D contiene una serie di sprite. Principalmente conta quello superiore.

Nel modulo regole del gioco, ho un sacco di codice come questo:

public boolean isGameWon(Board board) { 
    for (Point point : board.getTargetPoints()) 
     if(!(board.getTopSpriteAt(point) instanceof Box)) 
      return false; 
    return true; 
} 

---

Upadate://Do something conta se c'è un Box in cima ad ogni Target. Non vedo come ciò possa essere fatto semplicemente aggiungendo doSomething() a Sprite, a meno che doSomething() restituisca 1 se lo sprite è una scatola e 0 altrimenti. (e questo sarebbe esattamente lo stesso di instanceof).

---

So che instanceof è considerato dannoso perché elimina l'idea della programmazione orientata agli oggetti.

Tuttavia, non sono sicuro di come correggere il codice nel mio caso. Ecco alcuni pensieri che ho avuto:

• Non credo lo rende meglio per semplice aggiungere un metodo isABox() all'interfaccia Sprite.
• Sarebbe d'aiuto se Box fosse un'interfaccia, quindi altre classi potrebbero ottenere lo stesso privilegio?
• Dovrei provare a fare qualcosa di fantasia come la corrispondenza di modelli/doppia spedizione, con modelli simili ai visitatori?
• Va bene che il modulo delle regole funziona intimamente con i tipi, semplicemente perché si suppone che conoscano comunque la loro semantica?
• L'intera idea di un modello di strategia del modulo regole è difettosa?
• Non ha senso costruire le regole negli Sprites, poiché dovrebbero essere tutti modificati dopo l'aggiunta di un nuovo tipo.

Spero che tu abbia provato qualcosa di simile e riesca a indicarmi la giusta direzione.

Answer 1

Ecco la mia prova. Prendere in considerazione la definizione di un enum con i diversi tipi Sprite:

class Sprite { 
    public enum SpriteType { 
     BOX, CAT, BOTTLE, HUMAN, TARGET, /* ... */, SIMPLE; 
    } 

    public SpriteType getSpriteType(){ 
     return SIMPLE; 
    } 
} 


class Box extends Sprite { 
    @Override 
    public SpriteType getSpriteType(){ 
     return Sprite.SpriteType.BOX; 
    } 
} 

E infine:

public boolean isGameWon(Board board) { 
    for (Point point : board.getTargetPoints()) 
     if(board.getTopSpriteAt(point).getSpriteType()!=SpriteType.BOX) 
      return false; 
    return true; 
} 

In questo modo, è possibile risolvere il problema di dover creare un metodo isATypeX() in Sprite per ogni tipo X. Se è necessario un nuovo tipo, si aggiunge un nuovo valore all'enumerazione e solo la regola che deve controllare questo tipo dovrà riconoscerla.

Answer 2

Uso polymorphism:

class Sprite { 
    .. 
    someMethod(){ 
    //do sprite 
    } 
    .. 
} 

class Box extends Sprite { 
    .. 
    @Overrides 
    someMethod(){ 
    //do box 
    } 
    .. 
} 

Quindi, basta chiamare sprite.someMethod() nel tuo esempio.

Answer 3

Le dichiarazioni generali sulla progettazione orientata agli oggetti/i refactoring sono difficili da fornire a IMHO poiché l'azione "migliore" dipende molto dal contesto.

Si dovrebbe provare a spostare il "Fai qualcosa" in un metodo virtuale di Sprite che non fa nulla. Questo metodo può essere chiamato dal tuo loop.

La casella può quindi sovrascriverla e fare il "qualcosa".

Answer 4

L'overloading di base è il modo per andare qui. E 'la gerarchia delle classi Sprite che dovrebbe sapere cosa fare e come farlo, come in:

interface Sprite { 
    boolean isCountable(); 
} 


class MyOtherSprite implements Sprite { 
    boolean isCountable() { 
     return false; 
    } 
} 

class Box implements Sprite { 
    boolean isCountable() { 
     return true; 
    } 
} 

int count = 0; 
for (Point point : board.getTargetPoints()) { 
    Sprite sprite = board.getTopSpriteAt(point); 
    count += sprite.isCountable() ? 1 : 0; 
} 

EDIT: La modifica alla domanda non cambia radicalmente il problema.Quello che hai è una logica che è applicabile solo a Box. Ancora una volta, incapsula quella particolare logica nell'istanza Box (vedi sopra). Puoi andare oltre e creare una superclasse generica per i tuoi sprite che definisce un valore predefinito per isCountable() (nota che il metodo è simile a quello di isBox ma in realtà è migliore dal punto di vista del design, poiché non ha senso che un cerchio abbia un metodo isBox - la casella dovrebbe contenere anche un metodo isCircle?).

Answer 5

Quando si dice "stack" e "quello principale conta", non si può prendere quello superiore e fare qualcosa con esso?

Answer 6

Fondamentalmente, invece di

if (sprite instanceof Box) 
    // Do something 

Uso

sprite.doSomething() 

dove doSomething() è definito nella Sprite e sovrascritto in Box.

Se si desidera che queste regole separate dalla gerarchia classe Sprite, è possibile passare ad un Rules classe separata (o interfaccia), dove Sprite ha un metodo getRules() e sottoclassi di ritorno diverse implementazioni. Ciò aumenterebbe ulteriormente la flessibilità, poiché consente agli oggetti della stessa sottoclasse Sprite di avere un comportamento diverso.

Answer 7

Penso che ciò che le persone ti hanno suggerito sia corretto. Il tuo doSomething() può assomigliare a questo:

class Sprite { 
    public int doSomething(int cnt){ 
     return cnt; 
    } 
} 

class Box extends Sprite { 
    @Override 
    public int doSomething(int cnt){ 
     return cnt + 1; 
    } 
} 

Quindi nel tuo codice originale, si può fare questo:

int cnt = 0; 
for (Point point : board.getTargetPoints()) { 
    Sprite sprite = board.getTopSpriteAt(point) 
    cnt = sprite.doSomething(cnt); 
} 

Oppure, si può anche raggiungere lo stesso obiettivo con quello che lei ha suggerito, ma può costare un calcolo aggiuntivo per ciclo.

Answer 8

Ecco un esempio di contatore generico senza un metodo isAnX() per ogni tipo che si desidera contare. Supponi di voler contare il numero di tipo X sulla lavagna.

public int count(Class type) { 
    int count = 0; 
    for (Point point : board.getTargetPoints()) 
     if(type.isAssignable(board.getTopSpriteAt(point))) 
      count++; 
    return count; 
} 

---

ho il sospetto che cosa si vuole veramente è

public boolean isAllBoxes() { 
    for (Point point : board.getTargetPoints()) 
     if(!board.getTopSpriteAt(point).isABox()) 
      return false; 
    return true; 
} 

Answer 9

Quello che stai veramente testando qui è,

il giocatore può vincere la partita con questo Sprite nella parte superiore della scheda?

Pertanto, propongo questi nomi:

public boolean isGameWon(Board board) { 
    for (Point point : board.getTargetPoints()) 
     if(!board.getTopSpriteAt(point).isWinningSprite()) 
      return false; 
    return true; 
} 

Non c'è assolutamente alcun senso avere una funzione isBox. Assolutamente no. Si potrebbe anche usare instanceof.

Ma se Box, Bottle e Target sono tutte le tessere vincenti, allora si può avere tutti tornare

class Box { 
    public override bool isWinningSprite() { return true; } 
} 

È quindi possibile aggiungere un altro tipo di sprite "vincente" senza alterare la funzione isGameWon.

Answer 10

Instanceof: (quasi) sempre Nocivo

ho preso uno sguardo a tutte le risposte al tuo post e cercato di capire che cosa stavate facendo. E sono giunto alla conclusione che instanceof è esattamente quello che vuoi e il tuo esempio di codice originale andava bene.

Lei ha chiarito che:

• Sei non violare il principio di sostituzione Liskov dal momento che nessuno del codice Box invalida il codice Sprite.

• non biforcazione del codice con risposta a instanceof. Questo è il motivo per cui la gente dice che instanceof è cattivo; perché le persone lo fanno:

  if(shape instanceof Circle) { 
      area = Circle(shape).circleArea(); 
  } else if(shape instanceof Square) { 
      area = Square(shape).squareArea(); 
  } else if(shape instanceof Triangle) { 
      area = Triangle(shape).triangleArea(); 
  } 
  

  Questo è il motivo per cui le persone evitano instanceof. Ma questo non è quello che stai facendo.

• Esiste una relazione uno-a-uno tra ilBox e vincere la partita (nessun altro sprite possono vincere la partita). Quindi non hai bisogno di un'ulteriore astrazione sprite "vincente" (perché Boxes == Vincitori).

• si sono semplicemente controllando la scheda per assicurarsi che ogni elemento superiore è una scatola. Questo è esattamente ciò che è stato progettato per fare instanceof.

La risposta di tutti gli altri (inclusa la mia) aggiunge un ulteriore meccanismo per verificare se uno Sprite è una scatola. Tuttavia non aggiungono alcuna robustezza. In effetti stai prendendo funzionalità che sono già fornite dalla lingua e reimplementandole nel tuo codice.

Tomas Narros sostiene che è necessario distinguere il proprio codice tra "tipi semantici" e "tipi java". Non sono d'accordo. Hai già stabilito di disporre di un tipo java, Box, che sottoclasse Sprite. Quindi hai già tutte le informazioni di cui hai bisogno.

Dal mio punto di vista, avere un secondo meccanismo indipendente che riporta anche "I am a Box", viola DRY (Do not Repeat Yourself). Questo significa non avere due fonti indipendenti per la stessa informazione. Ora devi mantenere un enum e una struttura di classe.

Il cosiddetto "vantaggio" è la capacità di piroettare attorno a una parola chiave che soddisfa pienamente lo scopo ed è dannosa se utilizzata in modi più dannosi.

---

La regola d'oro è usare la testa. Non obbedire alle regole come un fatto difficile. Interrogali, impara perché sono lì e piegali quando è appropriato.

Answer 11

Come utilizzare un visitatore.

Ogni punti eredita il metodo acceptBoxDetection:

boolean acceptBoxDetection(Visitor boxDetector){ 
      return boxDetector.visit(this); 
} 

and then Visitor does: 

boolean visit(Box box){ 
     return true; 
} 

boolean visit(OtherStuff other){ 
     return false; 
}