2012-01-03 43 views
8

En un juego basado en sprites que estoy escribiendo, cada campo en una cuadrícula 2D contiene una pila de sprites. En su mayoría, el primero cuenta.Deshacerse de `instanceof`

En el módulo de reglas del juego, que tienen una gran cantidad de código como este:

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

upadate://Do something recuento si hay un Box en la parte superior de cada Target. No veo cómo hacerlo con solo agregar doSomething() a Sprite, a menos que doSomething() devuelva 1 si el sprite es una caja y 0 en caso contrario. (y eso sería lo mismo que instanceof).


Sé que instanceof se considera perjudicial porque elimina la idea de la programación orientada a objetos.

Sin embargo, no estoy seguro de cómo corregir el código en mi caso. Aquí están algunos pensamientos que he tenido:

  • No creo que hace que sea mejor a la simple añadir un método isABox() a la interfaz Sprite.
  • ¿Ayudaría si Box fuera una interfaz, por lo que otras clases podrían obtener el mismo privilegio?
  • ¿Debo intentar hacer algo elegante como la coincidencia de patrones/despacho doble, con patrones de visitante como?
  • ¿Está bien que el módulo de reglas funcione íntimamente con los tipos, simplemente porque se supone que conoce su semántica de todos modos?
  • ¿Es defectuosa la idea de un patrón de estrategia de módulo de reglas?
  • No tiene sentido crear las reglas en los Sprites, ya que todas tendrían que cambiarse cuando se agregue un nuevo tipo.

Espero que haya intentado algo similar y pueda orientarme en la dirección correcta.

+0

Qué le pareció la adición de un método doSomthing() a la interfaz de Sprite y que cada clase impleneting proporcionar una implementación para ello? – A4L

+1

Considere que una cuadrícula de referencias de clase base no es apropiada aquí, porque desea tratar especialmente clases derivadas específicas. (Básicamente, estás violando el [LSP] (http://en.wikipedia.org/wiki/Liskov_substitution_principle)). –

+0

@OliCharlesworth Creo que has dado en el clavo, solo que no he encontrado otras formas? –

Respuesta

3

Aquí está mi intento. Considere definir una enumeración con los diferentes tipos de 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; 
    } 
} 

Y por último:

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

De esta manera, se puede resolver el problema de tener que crear un método isATypeX() en Sprite para cada tipo X. Si necesita un tipo nuevo, agrega un nuevo valor a la enumeración, y solo la regla que necesita verificar este tipo deberá conocerlo.

+1

Buen intento. Esto es agradable porque las enumeraciones desempeñan el papel de identificadores semánticos a los ojos de 'Reglas', que de alguna manera separa la semántica de los tipos. Pero, ¿realmente le da algo al código en términos de mantenibilidad? Tendré que pensar en esto por un momento :) –

+0

Creo que sí, ya que crear un nuevo tipo solo afecta la enumeración, el tipo en sí y las reglas a aplicar. –

+1

Eso sigue siendo uno más que solo usar el tipo de clase como el tipo semántico también;) –

7

Uso polymorphism:

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

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

Por lo tanto, sólo tiene que llamar sprite.someMethod() en su ejemplo.

+0

Agregué una actualización para mostrar por qué esto no funcionará. –

+0

Por favor, muestre algunos ejemplos de código. – Artem

+0

¿Qué tal ahora? No veo cómo 'someMethod()' puede ser cualquier cosa que no sea 'int isABox()'? –

0

Declaraciones generales sobre el diseño orientado a objetos/refactorizaciones son difíciles de dar en mi humilde opinión, ya que la "mejor" acción depende mucho del contexto.

Debería intentar mover el "Hacer algo" en un método virtual de Sprite que no hace nada. Este método se puede llamar desde dentro de su ciclo.

Box puede anularlo y hacer el "algo".

3

Sobrecarga básica es el camino a seguir aquí. Es la jerarquía de clases Sprite que debe saber qué hacer y cómo hacerlo, como en:

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: Tu edición de la cuestión no cambia fundamentalmente el problema.Lo que tienes es una lógica que solo se aplica al Box. Nuevamente, encapsule esa lógica particular en la instancia Box (vea arriba). Podrías ir más allá y crear una superclase genérica para tus sprites que defina un valor predeterminado para isCountable() (ten en cuenta que el método es similar al de isBox pero es mejor desde el punto de vista del diseño, ya que no tiene sentido que un círculo tenga una Método isBox - ¿Debería el cuadro también contener un método isCircle?

-1

Cuando dices 'stack' y 'the top one counts', ¿no podrías simplemente tomar el superior y hacer algo con él?

+0

Eso es lo que 'getTopSpriteAt' hace. Toma algo de la parte superior de la pila. Acabo de agregarlo allí, porque podría ser útil en una solución de coincidencia de patrones. –

+0

Vaya, mi culpa. Bueno, sugiero ir con la respuesta de @Artems. – DerMike

1

Básicamente, en lugar de

if (sprite instanceof Box) 
    // Do something 

Uso

sprite.doSomething() 

donde doSomething() se define en en Sprite y anulado en Box.

Si desea que estas reglas separadas de la jerarquía de clases Sprite, puede moverlos a una Rules clase (o interfaz), donde Sprite tiene un método getRules() y subclases devolver diferentes implementaciones. Esto aumentaría aún más la flexibilidad, ya que permite que los objetos de la misma subclase Sprite tengan un comportamiento diferente.

+0

¿Cómo implementarías la prueba "¿Hay un cuadro en cada objetivo" usando las reglas guardadas dentro de los sprites? –

+0

@Thomas Ahle: 'cnt + = sprite.getTargetCount()', donde 'getTargetCount()' devuelve 1 en 'Box' y 0 en 'Sprite'. Puede haber soluciones más limpias, pero eso viene a la mente de inmediato. Y no, eso no es lo mismo que tener una instancia de prueba en el código que hace el recuento. –

+0

Sé a dónde quiere ir, pero ¿cómo describiría ese método en los documentos? "Devuelve 1 si es un cuadro, 0 de lo contrario?". Puede que no sea lo mismo que "instanceof", pero ¿cómo es mejor? –

0

Creo que lo que la gente le sugirió es correcto. Su doSomething() puede tener este aspecto:

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

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

Así que en su código original, usted puede hacer esto:

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

O de lo contrario, también se puede lograr el mismo objetivo con lo que usted sugiere, pero que puede costar un cálculo adicional por ciclo.

class Sprite { 
    public boolean isBox() { 
     return false; 
    } 
} 

class Box extends Sprite { 
    @Override 
    public boolean isBox(){ 
     return true; 
    } 
} 
+0

Pero esto es exactamente lo mismo que tener un 'boolean isBox()' en Sprite, que es exactamente lo mismo que tener 'instanceof Box' .. Supongo que el problema aquí está en las condiciones previas donde quiero saber algo sobre Boxes y Objetivos Esto es sintaxis en lugar de semántica y viola ese principio de Liskov. Simplemente no sé qué más hacer? –

+0

Bueno, no es exactamente lo mismo. Si anulas el método 'doSomething' en el cuadro, cuando ejecutas ese método desde cualquier objeto Sprite que sea un cuadro, activará el método dentro de la clase Box. No necesita un cálculo adicional para 'isBox()' o 'instanceof'. Además, no estás violando la regla de Liskov si anulas el método 'doSomething'. –

+0

El principio de sustitución Liskov dice que 'si S es un subtipo de T, entonces los objetos de tipo T pueden reemplazarse con objetos de tipo S sin alterar ninguna de las propiedades deseables de ese programa (corrección, tarea realizada, etc.)'. Las 'propiedades deseables' de tu programa son incrementar' cnt' si y solo si el 'Sprite' es un' Box'. Entonces, si sustituyes todo tu 'Sprite' con su subclase y aún obtienes las mismas 'propiedades deseables', no estás violando el principio de Liskov. –

1

Aquí hay un ejemplo de un contador genérico sin un método isAnX() para cada tipo que desee contar. Digamos que quieres contar el número de tipo X en el tablero.

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

sospecho que lo que realmente quiere es

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

Ah sí, eso es mucho más limpio :) Todavía no resuelve el problema de tener que crear un método' isATypeX() ' en 'Sprite' para cada tipo X que creo (y las reglas deben conocerse). –

+0

@ThomasAhle Tal vez el ejemplo de conteo que he dado evita la necesidad de muchos métodos 'isAnXxx()'. –

+0

Y luego, en mis Reglas, tendría 'boolean isGameWon (Board board) {return count (Box.class) == board.getTargetPoints();}'? –

1

Lo que realmente está realizando una prueba de aquí es,

¿Puede el jugador ganar el juego con este Sprite en la parte superior de la junta?

Por lo tanto, sugieren que estos nombres:

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

No hay absolutamente ninguna razón para tener una función isBox. Ninguno en absoluto. También podría usar instanceof.

Pero si Box, Bottle y Target son todas las fichas ganadoras, entonces usted puede tener a todos volver

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

A continuación, puede añadir otro tipo de "ganar" sprites sin alterar la función isGameWon.

+1

ps No estoy al 100% sobre el nombre 'isWinningSprite', puede haber un nombre mejor dependiendo de qué juego está haciendo. –

+0

Estoy de acuerdo contigo, pero en realidad esto es lo que Tomas Narros responde también. El enum tipo "Box" bien podría llamarse el "sprite ganador". Su significado es solo a los ojos de las reglas. Lo que es importante es la diferencia entre los tipos de clase y los tipos de reglas. –

+0

@Thomas: Si no hay posibilidad de que haya otros sprites que sean un sprite ganador, aparte de Boxes, entonces creo que la solución más limpia y más fácil de mantener podría ser el uso de 'instanceof'. –

4

Instanceof: (casi) siempre es perjudicial

Me tomó un vistazo a todas las respuestas a sus correos y trataba de entender lo que estaba haciendo. Y he llegado a la conclusión de que instanceof es exactamente lo que quiere y su muestra del código original estaba bien.

Se aclaró que:

  • Usted no son violar el principio de sustitución Liskov ya que ninguno de código Caja invalida el código de Sprite.

  • Usted es no tecleando el código con la respuesta a instanceof. Esta es la razón por la que la gente dice que la instancia es mala; porque la gente hace esto:

    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(); 
    } 
    

    Esta es la razón por la cual las personas evitan instanceof. Pero esto no es lo que estás haciendo.

  • Existe una relación uno-a-uno entreBox y ganar el juego (no hay otros sprites pueden ganar el juego). Por lo tanto, no necesita una abstracción de sprites "ganadora" adicional (porque Boxes == Winners).

  • que son simplemente comprobar la tarjeta para asegurarse de que cada elemento superior es una caja. Esto es exactamente lo que instanceof está diseñado para hacer.

La respuesta de todos los demás (incluida la mía) agrega un mecanismo adicional para comprobar si un Sprite es una Caja. Sin embargo, no agregan ninguna solidez. De hecho, usted está tomando características que ya están provistas por el idioma y las está reimplementando en su propio código.

Tomas Narros argumenta que debe distinguir en su código entre "tipos semánticos" y "tipos de Java". Estoy en desacuerdo. Ya ha establecido que tiene un tipo de Java, Box, cuyas subclases Sprite. Entonces ya tiene toda la información que necesita.

En mi opinión, tener un segundo mecanismo independiente que también informa "Soy una caja", viola DRY (No repetir). Esto significa no tener dos fuentes independientes para la misma información. Ahora debe mantener un enum y una estructura de clase.

El llamado "beneficio" es la capacidad de hacer piruetas en torno a una palabra clave que cumple por completo el propósito, y es perjudicial cuando se usa de formas más dañinas.


La regla de oro es usar la cabeza . No obedezcas las reglas como un hecho difícil. Pregúnteles, aprenda por qué están allí y dóblelos cuando sea apropiado.

+0

http://www.codinghorror.com/blog/2009/02/real-ultimate-programming-power.html –

+0

http://www.javapractices.com/topic/TopicAction.do?Id=31 –

+1

Gran respuesta. Realmente lo aprecio. Lo que voy a hacer es probar la división de fundición/semántica y ver si hay suficiente diferencia para no violar DRY. O si me encuentro confundido sobre qué tipo de tipo usar una y otra vez. –

1

¿Qué le parece usar un visitante.

Cada puntos hereda el método 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; 
}