¿Demasiadas declaraciones 'si'?

263

El siguiente código funciona como lo necesito, pero es feo, excesivo o varias otras cosas. Miré las fórmulas e intenté escribir algunas soluciones, pero termino con una cantidad similar de declaraciones.

¿Hay algún tipo de fórmula matemática que me beneficiaría en este caso o son 16 si las declaraciones son aceptables?

Para explicar el código, es para una especie de juego basado en turnos simultáneos ... dos jugadores tienen cuatro botones de acción cada uno y los resultados provienen de una matriz (0-3), pero las variables 'uno' y 'dos' pueden ser asignado cualquier cosa si esto ayuda. El resultado es, 0 = ninguno gana, 1 = p1 gana, 2 = p2 gana, 3 = ambos ganan.

public int fightMath(int one, int two) {

    if(one == 0 && two == 0) { result = 0; }
    else if(one == 0 && two == 1) { result = 0; }
    else if(one == 0 && two == 2) { result = 1; }
    else if(one == 0 && two == 3) { result = 2; }
    else if(one == 1 && two == 0) { result = 0; }
    else if(one == 1 && two == 1) { result = 0; }
    else if(one == 1 && two == 2) { result = 2; }
    else if(one == 1 && two == 3) { result = 1; }
    else if(one == 2 && two == 0) { result = 2; }
    else if(one == 2 && two == 1) { result = 1; }
    else if(one == 2 && two == 2) { result = 3; }
    else if(one == 2 && two == 3) { result = 3; }
    else if(one == 3 && two == 0) { result = 1; }
    else if(one == 3 && two == 1) { result = 2; }
    else if(one == 3 && two == 2) { result = 3; }
    else if(one == 3 && two == 3) { result = 3; }

    return result;
}
TomFirth
fuente
1
@waqaslam: - ¿Esto puede ayudar a Java a cambiar la declaración para manejar dos variables?
Rahul Tripathi
9
¿Seguramente hay alguna lógica aquí que se puede generalizar en lugar de forzar bruta? ¿Seguramente hay alguna función f(a, b)que produce la respuesta en el caso general? No ha explicado la lógica del cálculo, por lo tanto, todas las respuestas son solo lápiz labial en un cerdo. Comenzaría por repensar seriamente la lógica de su programa, el uso de intindicadores para las acciones está muy desactualizado. enumLos s pueden contener lógica y son descriptivos, esto le permitiría escribir su código de una manera más moderna.
Boris the Spider
Después de leer las respuestas proporcionadas por @Steve Benett en su pregunta alternativa vinculada anteriormente, puedo suponer que no hay una respuesta de fórmula directa a esto, ya que es esencialmente lo mismo que una base de datos. Intenté explicar en la pregunta original que estaba haciendo un juego simple (luchador) y los usuarios tienen una selección de 4 botones: blockHigh (0), blockLow (1), attackHigh (2) y attackLow (3). Estos números se mantienen en una matriz hasta que se necesiten. Más tarde son utilizados por la función 'fightMath ()' que llama a las selecciones de playerOne contra playerTwos para dar el resultado. No hay detección de colisión real.
TomFirth
9
Si tiene una respuesta, publíquela como tal. La discusión extendida en los comentarios es difícil de seguir, especialmente cuando el código está involucrado. Si desea hablar sobre si esta pregunta debería haberse migrado a Revisión de código, hay una Meta discusión al respecto.
George Stocker
1
¿Qué quiere decir con "lo mismo que una base de datos"? Si estos valores están en la base de datos, extráigalos desde allí. De lo contrario, si es realmente tan complejo, lo dejaría como lo tiene y agregaría comentarios de lógica de negocios después de cada línea para que la gente entienda lo que está sucediendo. Es mejor (para mí) largo y explícito: alguien en el futuro puede entender lo que está sucediendo. Si lo coloca en un mapa o intenta guardar 8 líneas de código, la ventaja es realmente pequeña, y la reducción de tamaño es mayor: lo hace cada vez más confuso para alguien que necesita leer su código algún día.
skaz

Respuestas:

600

Si no puede encontrar una fórmula, puede usar una tabla para un número tan limitado de resultados:

final int[][] result = new int[][] {
  { 0, 0, 1, 2 },
  { 0, 0, 2, 1 },
  { 2, 1, 3, 3 },
  { 1, 2, 3, 3 }
};
return result[one][two];
laalto
fuente
77
Esto es interesante ya que no he visto esta solución antes. No estoy muy seguro de entender el resultado, pero disfrutaré probando eso.
TomFirth
44
No necesita la afirmación, Java arrojará un de IndexOutOfBoundsExceptiontodos modos si uno o más índices están fuera de límites.
JAB
43
@JoeHarper Si quieres algo fácil de leer, no utilizarás números mágicos en primer lugar y tendrás un comentario explicando el mapeo. Tal como está, prefiero esta versión a la original, pero para algo mantenible a largo plazo, usaría un enfoque que involucra tipos enumerados o al menos constantes con nombre.
JAB
13
@JoeHarper "Teóricamente" es una cosa, "prácticamente" es otra. Por supuesto, trato de usar nombres descriptivos (con la excepción de la convención i/ j/ kpara variables de bucle), constantes nombradas, organizar mi código de manera legible, etc., pero cuando los nombres de variables y funciones comienzan a ocupar más de 20 caracteres cada vez que lo encuentro en realidad conduce a un código menos legible. Mi enfoque habitual es tratar de obtener un código comprensible pero sucinto con comentarios aquí y allá para explicar por qué el código está estructurado de la manera en que está (frente a cómo). Poner el por qué en los nombres simplemente abarrota todo.
JAB
13
Me gusta esta solución para este problema específico porque los resultados en realidad ESTÁN dictados por una matriz de resultados.
Intentando el
201

Como su conjunto de datos es tan pequeño, puede comprimir todo en 1 entero largo y convertirlo en una fórmula

public int fightMath(int one,int two)
{
   return (int)(0xF9F66090L >> (2*(one*4 + two)))%4;
}

Variante más bit a bit:

Esto hace uso del hecho de que todo es un múltiplo de 2

public int fightMath(int one,int two)
{
   return (0xF9F66090 >> ((one << 3) | (two << 1))) & 0x3;
}

El origen de la constante mágica

¿Qué puedo decir? El mundo necesita magia, a veces la posibilidad de algo requiere su creación.

La esencia de la función que resuelve el problema de OP es un mapa de 2 números (uno, dos), dominio {0,1,2,3} al rango {0,1,2,3}. Cada una de las respuestas ha abordado cómo implementar ese mapa.

Además, puede ver en una serie de respuestas una reexpresión del problema como un mapa de 1 número de base 4 de 2 dígitos N (uno, dos) donde uno es el dígito 1, dos es el dígito 2 y N = 4 * uno + dos; N = {0,1,2, ..., 15} - dieciséis valores diferentes, eso es importante. La salida de la función es un número base 4 de 1 dígito {0,1,2,3} - 4 valores diferentes, también importantes.

Ahora, un número de base 4 de 1 dígito se puede expresar como un número de base 2 de 2 dígitos; {0,1,2,3} = {00,01,10,11}, por lo que cada salida puede codificarse con solo 2 bits. Desde arriba, solo hay 16 salidas diferentes posibles, por lo que 16 * 2 = 32 bits es todo lo que se necesita para codificar todo el mapa; Todo esto puede caber en 1 entero.

La constante M es una codificación del mapa m donde m (0) se codifica en bits M [0: 1], m (1) se codifica en bits M [2: 3] y m (n) se codifica en bits M [n * 2: n * 2 + 1].

Todo lo que queda es indexar y devolver la parte derecha de la constante, en este caso puede cambiar M a la derecha 2 * N veces y tomar los 2 bits menos significativos, es decir (M >> 2 * N) y 0x3. Las expresiones (una << 3) y (dos << 1) simplemente multiplican las cosas mientras observan que 2 * x = x << 1 y 8 * x = x << 3.

WaTeim
fuente
79
inteligente, pero nadie más que lea el código tendrá la oportunidad de entenderlo.
Aran Mulholland
106
Creo que esta es una práctica extremadamente mala. Nadie más que el autor lo entenderá. Desea mirar un fragmento de código y comprenderlo rápidamente. Pero esto es solo una pérdida de tiempo.
Balázs Németh
14
Estoy con @ BalázsMáriaNémeth en esto. Aunque es muy impresionante, ¡deberías estar codificando para psicópatas violentos!
OrhanC1
90
Todos los votantes negativos piensan que este es un olor a código horrible. Todos los votantes votan lo mismo, pero admiran la inteligencia detrás de esto. +1 (nunca use este código)
usr
44
¡Qué hermoso ejemplo de código de solo escritura !
lealand
98

No me gusta ninguna de las soluciones presentadas, excepto las de JAB. Ninguno de los otros facilita la lectura del código y la comprensión de lo que se está calculando .

Así es como escribiría este código: solo conozco C #, no Java, pero entiendes la imagen:

const bool t = true;
const bool f = false;
static readonly bool[,] attackResult = {
    { f, f, t, f }, 
    { f, f, f, t },
    { f, t, t, t },
    { t, f, t, t }
};
[Flags] enum HitResult 
{ 
    Neither = 0,
    PlayerOne = 1,
    PlayerTwo = 2,
    Both = PlayerOne | PlayerTwo
}
static HitResult ResolveAttack(int one, int two)
{
    return 
        (attackResult[one, two] ? HitResult.PlayerOne : HitResult.Neither) | 
        (attackResult[two, one] ? HitResult.PlayerTwo : HitResult.Neither);
}    

Ahora está mucho más claro lo que se está calculando aquí: esto enfatiza que estamos calculando quién es golpeado por qué ataque y devolviendo ambos resultados.

Sin embargo, esto podría ser aún mejor; esa matriz booleana es algo opaca. Me gusta el enfoque de búsqueda en la tabla, pero me inclinaría a escribirlo de tal manera que dejara en claro cuáles eran las semánticas del juego. Es decir, en lugar de "un ataque de cero y una defensa de uno da como resultado ningún golpe", en su lugar, encuentra una manera de hacer que el código implique más claramente "un ataque de patada baja y una defensa de bloque bajo no da ningún golpe". Haz que el código refleje la lógica de negocios del juego.

Eric Lippert
fuente
66
Disparates. La mayoría de los programas con poca experiencia podrán apreciar los consejos que se están dando aquí y aplicar el estilo de codificación a su propio idioma. La pregunta era cómo evitar una cadena de ifs. Esto muestra cómo.
GreenAsJade
66
@ user3414693: Soy muy consciente de que es una pregunta de Java. Si lees la respuesta cuidadosamente, eso queda claro. Si crees que mi respuesta es imprudente, te animo a que escribas tu propia respuesta que te guste más.
Eric Lippert
1
@EricLippert También me gusta la solución de JAB. En mi humilde opinión, el tipo de enumeración en C # deja mucho que desear. No sigue el pozo de la filosofía del éxito que hacen el resto de las características. Por ejemplo, stackoverflow.com/a/847353/92414 ¿Tiene el equipo de C # algún plan para crear un nuevo tipo de enumeración (para evitar romper el código existente) que está mejor diseñado?
SoluciónYogi
@SolutionYogi: tampoco me gustan mucho las enumeraciones en C #, aunque son como son por buenas razones históricas. (Principalmente por compatibilidad con las enumeraciones COM existentes.) No conozco ningún plan para agregar nuevos equipos para enumeraciones en C # 6.
Eric Lippert
3
@Lista no, los comentarios no se ejecutan. OP hizo exactamente lo que debería hacerse; convertir comentarios a código claro. Ver, por ejemplo, Steve McConnell Code Complete stevemcconnell.com/cccntnt.htm
djechlin el
87

Puedes crear una matriz que contenga resultados

int[][] results = {{0, 0, 1, 2}, {0, 0, 2, 1},{2, 1, 3, 3},{2, 1, 3, 3}};

Cuando quieras obtener valor, usarás

public int fightMath(int one, int two) {
  return this.results[one][two]; 
}
djm.im
fuente
69

Otras personas ya han sugerido mi idea inicial, el método de matriz, pero además de consolidar las declaraciones if, puede evitar algo de lo que tiene asegurándose de que los argumentos proporcionados estén en el rango esperado y utilizando retornos en el lugar (algunos códigos Los estándares que he visto imponen un punto de salida para las funciones, pero he encontrado que los retornos múltiples son muy útiles para evitar la codificación de flechas y, con la prevalencia de excepciones en Java, no tiene mucho sentido hacer cumplir estrictamente dicha regla de todos modos ya que cualquier excepción no detectada lanzada dentro del método es un posible punto de salida de todos modos). La posibilidad de anidar declaraciones de cambio es una posibilidad, pero para el pequeño rango de valores que está comprobando aquí, encuentro si las declaraciones son más compactas y no es probable que generen una gran diferencia de rendimiento,

public int fightMath(int one, int two) {
    if (one > 3 || one < 0 || two > 3 || two < 0) {
        throw new IllegalArgumentException("Result is undefined for arguments outside the range [0, 3]");
    }

    if (one <= 1) {
        if (two <= 1) return 0;
        if (two - one == 2) return 1;
        return 2; // two can only be 3 here, no need for an explicit conditional
    }

    // one >= 2
    if (two >= 2) return 3;
    if (two == 1) return 1;
    return 2; // two can only be 0 here
}

Esto termina siendo menos legible de lo que podría ser debido a la irregularidad de partes del mapeo input-> result. En su lugar, prefiero el estilo de matriz debido a su simplicidad y a cómo puede configurar la matriz para que tenga sentido visualmente (aunque eso está en parte influenciado por mis recuerdos de los mapas de Karnaugh):

int[][] results = {{0, 0, 1, 2},
                   {0, 0, 2, 1},
                   {2, 1, 3, 3},
                   {2, 1, 3, 3}};

Actualización: dada su mención de bloqueo / golpe, aquí hay un cambio más radical en la función que utiliza tipos enumerados de propiedad / retención de atributos para las entradas y el resultado y también modifica un poco el resultado para dar cuenta del bloqueo, lo que debería dar como resultado Función legible.

enum MoveType {
    ATTACK,
    BLOCK;
}

enum MoveHeight {
    HIGH,
    LOW;
}

enum Move {
    // Enum members can have properties/attributes/data members of their own
    ATTACK_HIGH(MoveType.ATTACK, MoveHeight.HIGH),
    ATTACK_LOW(MoveType.ATTACK, MoveHeight.LOW),
    BLOCK_HIGH(MoveType.BLOCK, MoveHeight.HIGH),
    BLOCK_LOW(MoveType.BLOCK, MoveHeight.LOW);

    public final MoveType type;
    public final MoveHeight height;

    private Move(MoveType type, MoveHeight height) {
        this.type = type;
        this.height = height;
    }

    /** Makes the attack checks later on simpler. */
    public boolean isAttack() {
        return this.type == MoveType.ATTACK;
    }
}

enum LandedHit {
    NEITHER,
    PLAYER_ONE,
    PLAYER_TWO,
    BOTH;
}

LandedHit fightMath(Move one, Move two) {
    // One is an attack, the other is a block
    if (one.type != two.type) {
        // attack at some height gets blocked by block at same height
        if (one.height == two.height) return LandedHit.NEITHER;

        // Either player 1 attacked or player 2 attacked; whoever did
        // lands a hit
        if (one.isAttack()) return LandedHit.PLAYER_ONE;
        return LandedHit.PLAYER_TWO;
    }

    // both attack
    if (one.isAttack()) return LandedHit.BOTH;

    // both block
    return LandedHit.NEITHER;
}

Ni siquiera tiene que cambiar la función en sí si desea agregar bloques / ataques de más alturas, solo las enumeraciones; Sin embargo, agregar tipos adicionales de movimientos probablemente requerirá la modificación de la función. Además, EnumSets podría ser más extensible que usar enumeraciones adicionales como propiedades de la enumeración principal, por ejemplo, EnumSet<Move> attacks = EnumSet.of(Move.ATTACK_HIGH, Move.ATTACK_LOW, ...);y luego en attacks.contains(move)lugar de hacerlo move.type == MoveType.ATTACK, aunque el uso de EnumSets probablemente será un poco más lento que las comprobaciones de igualdad directa.


Para el caso donde un bloque exitoso resulta en un contador, puede reemplazarlo if (one.height == two.height) return LandedHit.NEITHER;con

if (one.height == two.height) {
    // Successful block results in a counter against the attacker
    if (one.isAttack()) return LandedHit.PLAYER_TWO;
    return LandedHit.PLAYER_ONE;
}

Además, reemplazar algunas de las ifdeclaraciones con el uso del operador ternario ( boolean_expression ? result_if_true : result_if_false) podría hacer que el código sea más compacto (por ejemplo, el código en el bloque anterior se volvería return one.isAttack() ? LandedHit.PLAYER_TWO : LandedHit.PLAYER_ONE;), pero eso puede dar lugar a líneas más difíciles de leer, por lo que no No lo recomiendo para ramificaciones más complejas.

PINCHAZO
fuente
Definitivamente voy a investigar esto, pero mi código actual me permite usar el valor int oney twoser reutilizado como puntos de inicio en mi hoja de sprites. Aunque no requiere mucho código adicional para permitir eso.
TomFirth
2
@ TomFirth84 Hay una EnumMapclase que se puede utilizar para mapear las enumeraciones de sus desplazamientos enteros (también se puede usar los valores ordinales de los miembros de enumeración directamente, por ejemplo, Move.ATTACK_HIGH.ordinal()sería 0, Move.ATTACK_LOW.ordinal()sería 1, etc., pero eso es más frágil / menos flexible que forma explícita asociar cada miembro con un valor como agregar valores de enumeración entre los existentes eliminaría el conteo, lo que no sería el caso con un EnumMap.)
JAB
77
Esta es la solución más fácil de leer, ya que traduce el código en algo significativo para la persona que lee el código.
David Stanley
Su código, al menos el que usa enumeraciones, es incorrecto. Según las declaraciones if en OP, un bloqueo exitoso conduce a un golpe en el atacante. Pero +1 para código significativo.
Taemyr
2
Incluso puede agregar un attack(against)método a la Moveenumeración, devolviendo HIT cuando el movimiento es un ataque exitoso, BACKFIRE cuando el movimiento es un ataque bloqueado y NADA cuando no es un ataque. De esta forma, puede implementarlo en general ( public boolean attack(Move other) { if this.isAttack() return (other.isAttack() || other.height != this.height) ? HIT : BACKFIRE; return NOTHING; }) y anularlo en movimientos específicos cuando sea necesario (movimientos débiles que cualquier bloque puede bloquear, ataques que nunca son contraproducentes, etc.)
reescrito el
50

¿Por qué no usar una matriz?

Comenzaré desde el principio. Veo un patrón, los valores van de 0 a 3 y desea capturar todos los valores posibles. Esta es su mesa:

0 & 0 = 0
0 & 1 = 0
0 & 2 = 1
0 & 3 = 2
1 & 0 = 0
1 & 1 = 0
1 & 2 = 2
1 & 3 = 1
2 & 0 = 2
2 & 1 = 1
2 & 2 = 3
2 & 3 = 3
3 & 0 = 2
3 & 1 = 1
3 & 2 = 3
3 & 3 = 3

Cuando miramos esta misma tabla binaria, vemos los siguientes resultados:

00 & 00 = 00
00 & 01 = 00
00 & 10 = 01
00 & 11 = 10
01 & 00 = 00
01 & 01 = 00
01 & 10 = 10
01 & 11 = 01
10 & 00 = 10
10 & 01 = 01
10 & 10 = 11
10 & 11 = 11
11 & 00 = 10
11 & 01 = 01
11 & 10 = 11
11 & 11 = 11

Ahora tal vez ya ve algún patrón, pero cuando combino el valor uno y dos, veo que está usando todos los valores 0000, 0001, 0010, ..... 1110 y 1111. Ahora combinemos el valor uno y dos para hacer un solo Entero de 4 bits.

0000 = 00
0001 = 00
0010 = 01
0011 = 10
0100 = 00
0101 = 00
0110 = 10
0111 = 01
1000 = 10
1001 = 01
1010 = 11
1011 = 11
1100 = 10
1101 = 01
1110 = 11
1111 = 11

Cuando traducimos esto nuevamente a valores decimales, vemos una matriz muy posible de valores donde el uno y dos combinados podrían usarse como índice:

0 = 0
1 = 0
2 = 1
3 = 2
4 = 0
5 = 0
6 = 2
7 = 1
8 = 2
9 = 1
10 = 3
11 = 3
12 = 2
13 = 1
14 = 3
15 = 3

La matriz es entonces {0, 0, 1, 2, 0, 0, 2, 1, 2, 1, 3, 3, 2, 1, 3, 3}, donde su índice es simplemente uno y dos combinados.

No soy un programador de Java, pero puedes deshacerte de todas las declaraciones if y simplemente escribirlo de esta manera:

int[] myIntArray = {0, 0, 1, 2, 0, 0, 2, 1, 2, 1, 3, 3, 2, 1, 3, 3};
result = myIntArray[one * 4 + two]; 

No sé si un desplazamiento de bits por 2 es más rápido que la multiplicación. Pero podría valer la pena intentarlo.

dj bazzie wazzie
fuente
2
Un cambio de bits de 2 es casi definitivamente más rápido que una multiplicación de 4. En el mejor de los casos, la multiplicación por 4 reconocería que 4 es 2 ^ 2 y haría un cambio de bits en sí mismo (traducido potencialmente por el compilador). Francamente, para mí, el cambio es más legible.
Cruncher
Me gusta tu enfoque! Básicamente, aplana una matriz 4x4 en una matriz de 16 elementos.
Cameron Tinker
66
Hoy en día, si no me equivoco, el compilador reconocerá indudablemente que estás multiplicando por una potencia de dos y lo optimizará en consecuencia. Entonces, para usted, el programador, el cambio de bits y la multiplicación deben tener exactamente el mismo rendimiento.
Tanner Swett
24

Esto usa un poco de bitmagic (ya lo estás haciendo manteniendo dos bits de información (bajo / alto y ataque / bloqueo) en un solo entero):

No lo he ejecutado, solo lo escribí aquí, por favor verifique dos veces. La idea seguramente funciona. EDITAR: ahora se ha probado para cada entrada, funciona bien.

public int fightMath(int one, int two) {
    if(one<2 && two<2){ //both players blocking
        return 0; // nobody hits
    }else if(one>1 && two>1){ //both players attacking
        return 3; // both hit
    }else{ // some of them attack, other one blocks
        int different_height = (one ^ two) & 1; // is 0 if they are both going for the same height - i.e. blocker wins, and 1 if height is different, thus attacker wins
        int attacker = one>1?1:0; // is 1 if one is the attacker, two is the blocker, and 0 if one is the blocker, two is the attacker
        return (attacker ^ different_height) + 1;
    }
}

¿O debería sugerir separar los dos bits de información en variables separadas? El código basado principalmente en operaciones de bits como este es generalmente muy difícil de mantener.

elias
fuente
2
Estoy de acuerdo con esta solución, se parece mucho a lo que tenía en mente en mi comentario sobre la pregunta principal. Preferiría dividirlo en variables separadas, lo que facilitaría agregar un ataque medio, por ejemplo, en el futuro.
Yoh
2
Acabo de corregir algunos errores en el código anterior después de probarlo, ahora funciona bien. Yendo más allá en la forma del manipulador de bits, también encontré una solución de una línea, que todavía no es tan mística como la máscara de bits en otras respuestas, pero aún es lo suficientemente difícil como para aturdir tu mente:return ((one ^ two) & 2) == 0 ? (one & 2) / 2 * 3 : ((one & 2) / 2 ^ ((one ^ two) & 1)) + 1;
elias
1
Esta es la mejor respuesta ya que cualquier programador nuevo que lo lea comprenderá realmente la magia que ocurre detrás de todos esos números mágicos.
bezmax
20

Para ser sincero, todos tienen su propio estilo de código. No hubiera pensado que el rendimiento se vería demasiado afectado. Si entiende esto mejor que usar una versión de caja de interruptor, continúe usándolo.

Podría anidar los ifs, por lo que potencialmente habría un ligero aumento en el rendimiento de sus últimas comprobaciones if, ya que no habría pasado por tantas declaraciones if. Pero en su contexto de un curso básico de Java probablemente no se beneficiará.

else if(one == 3 && two == 3) { result = 3; }

Entonces, en lugar de ...

if(one == 0 && two == 0) { result = 0; }
else if(one == 0 && two == 1) { result = 0; }
else if(one == 0 && two == 2) { result = 1; }
else if(one == 0 && two == 3) { result = 2; }

Tu harías ...

if(one == 0) 
{ 
    if(two == 0) { result = 0; }
    else if(two == 1) { result = 0; }
    else if(two == 2) { result = 1; }
    else if(two == 3) { result = 2; }
}

Y simplemente vuelva a formatearlo como prefiera.

Esto no hace que el código se vea mejor, pero creo que potencialmente lo acelera un poco.

Joe Harper
fuente
3
No sé si es realmente una buena práctica, pero para este caso, probablemente usaría instrucciones de cambio anidadas. Se necesitaría más espacio, pero sería realmente claro.
dyesdyes
Eso también funcionaría, pero supongo que es una cuestión de preferencia. De hecho, prefiero las declaraciones if, ya que prácticamente dice lo que está haciendo el código. No menospreciando tu opinión, por supuesto, lo que sea que funcione para ti :). ¡Vota por la sugerencia alternativa!
Joe Harper
12

Veamos que sabemos

1: sus respuestas son simétricas para P1 (jugador uno) y P2 (jugador dos). Esto tiene sentido para un juego de lucha, pero también es algo que puedes aprovechar para mejorar tu lógica.

2: 3 latidos 0 latidos 2 latidos 1 latidos 3. Los únicos casos no cubiertos por estos casos son combinaciones de 0 vs 1 y 2 vs 3. Para decirlo de otra manera, la tabla de victoria única se ve así: 0 latidos 2, 1 latidos 3, 2 latidos 1, 3 latidos 0.

3: Si 0/1 se enfrentan entre sí, entonces hay un empate sin golpes, pero si 2/3 se enfrentan entre sí, ambos golpean

Primero, construyamos una función unidireccional que nos diga si ganamos:

// returns whether we beat our opponent
public boolean doesBeat(int attacker, int defender) {
  int[] beats = {2, 3, 1, 0};
  return defender == beats[attacker];
}

Entonces podemos usar esta función para componer el resultado final:

// returns the overall fight result
// bit 0 = one hits
// bit 1 = two hits
public int fightMath(int one, int two)
{
  // Check to see whether either has an outright winning combo
  if (doesBeat(one, two))
    return 1;

  if (doesBeat(two, one))
    return 2;

  // If both have 0/1 then its hitless draw but if both have 2/3 then they both hit.
  // We can check this by seeing whether the second bit is set and we need only check
  // one's value as combinations where they don't both have 0/1 or 2/3 have already
  // been dealt with 
  return (one & 2) ? 3 : 0;
}

Si bien esto es posiblemente más complejo y probablemente más lento que la búsqueda en la tabla ofrecida en muchas respuestas, creo que es un método superior porque realmente encapsula la lógica de su código y lo describe a cualquiera que esté leyendo su código. Creo que esto lo hace una mejor implementación.

(Ha pasado un tiempo desde que hice cualquier Java, así que me disculpo si la sintaxis está desactivada, espero que todavía sea inteligible si me equivoco un poco)

Por cierto, 0-3 claramente significa algo; no son valores arbitrarios, por lo que sería útil nombrarlos.

Jack Aidley
fuente
11

Espero entender la lógica correctamente. ¿Qué tal algo como:

public int fightMath (int one, int two)
{
    int oneHit = ((one == 3 && two != 1) || (one == 2 && two != 0)) ? 1 : 0;
    int twoHit = ((two == 3 && one != 1) || (two == 2 && one != 0)) ? 2 : 0;

    return oneHit+twoHit;
}

Comprobar un golpe alto o un golpe bajo no está bloqueado y lo mismo para el jugador dos.

Editar: Algoritmo no se entendió completamente, "golpe" otorgado al bloquear que no me di cuenta (Thx elias):

public int fightMath (int one, int two)
{
    int oneAttack = ((one == 3 && two != 1) || (one == 2 && two != 0)) ? 1 : (one >= 2) ? 2 : 0;
    int twoAttack = ((two == 3 && one != 1) || (two == 2 && one != 0)) ? 2 : (two >= 2) ? 1 : 0;

    return oneAttack | twoAttack;
}
Chris
fuente
¡Manera de encontrar el gran resultado del golpeteo!
Chad
1
Me gusta el enfoque, pero me temo que esta solución carece por completo de la posibilidad de golpear al bloquear un ataque (por ejemplo, si uno = 0 y dos = 2, devuelve 0, pero se espera 1 de acuerdo con la especificación). Tal vez pueda trabajar en ello para hacerlo bien, pero no estoy seguro de que el código resultante siga siendo tan elegante, ya que esto significa que las líneas crecerán un poco más.
elias
No se dio cuenta de que se había otorgado un "hit" por bloqueo. Gracias por mencionarlo. Ajustado con una solución muy simple.
Chris
10

No tengo experiencia con Java, por lo que puede haber algunos errores tipográficos. Considere el código como pseudocódigo.

Yo iría con un simple interruptor. Para eso, necesitaría una evaluación de un solo número. Sin embargo, para este caso, desde 0 <= one < 4 <= 9y 0 <= two < 4 <= 9, podemos convertir ambas entradas a un int simple multiplicando onepor 10 y sumando two. Luego use un interruptor en el número resultante como este:

public int fightMath(int one, int two) {
    // Convert one and two to a single variable in base 10
    int evaluate = one * 10 + two;

    switch(evaluate) {
        // I'd consider a comment in each line here and in the original code
        // for clarity
        case 0: result = 0; break;
        case 1: result = 0; break;
        case 1: result = 0; break;
        case 2: result = 1; break;
        case 3: result = 2; break;
        case 10: result = 0; break;
        case 11: result = 0; break;
        case 12: result = 2; break;
        case 13: result = 1; break;
        case 20: result = 2; break;
        case 21: result = 1; break;
        case 22: result = 3; break;
        case 23: result = 3; break;
        case 30: result = 1; break;
        case 31: result = 2; break;
        case 32: result = 3; break;
        case 33: result = 3; break;
    }

    return result;
}

Hay otro método corto que solo quiero señalar como un código teórico. Sin embargo, no lo usaría porque tiene una complejidad adicional con la que normalmente no quieres lidiar. La complejidad adicional proviene de la base 4 , porque el conteo es 0, 1, 2, 3, 10, 11, 12, 13, 20, ...

public int fightMath(int one, int two) {
    // Convert one and two to a single variable in base 4
    int evaluate = one * 4 + two;

    allresults = new int[] { 0, 0, 1, 2, 0, 0, 2, 1, 2, 1, 3, 3, 1, 2, 3, 3 };

    return allresults[evaluate];
}

Realmente solo una nota adicional, en caso de que me falte algo de Java. En PHP haría:

function fightMath($one, $two) {
    // Convert one and two to a single variable in base 4
    $evaluate = $one * 10 + $two;

    $allresults = array(
         0 => 0,  1 => 0,  2 => 1,  3 => 2,
        10 => 0, 11 => 0, 12 => 2, 13 => 1,
        20 => 2, 21 => 1, 22 => 3, 23 => 3,
        30 => 1, 31 => 2, 32 => 3, 33 => 3 );

    return $allresults[$evaluate];
}
Francisco Presencia
fuente
Java no tiene lamdas antes de la 8va versión
Kirill Gamazkov
1
Esta. Para un número tan pequeño de entradas, usaría un interruptor con un valor compuesto (aunque puede ser más legible con un multiplicador mayor que 10, como 100 o 1000).
Medinoc
7

Dado que prefiere ifcondicionales anidados , aquí hay otra forma.
Tenga en cuenta que no usa el resultmiembro y no cambia ningún estado.

public int fightMath(int one, int two) {
    if (one == 0) {
      if (two == 0) { return 0; }
      if (two == 1) { return 0; }
      if (two == 2) { return 1; }
      if (two == 3) { return 2; }
    }   
    if (one == 1) {
      if (two == 0) { return 0; }
      if (two == 1) { return 0; }
      if (two == 2) { return 2; }
      if (two == 3) { return 1; }
    }
    if (one == 2) {
      if (two == 0) { return 2; }
      if (two == 1) { return 1; }
      if (two == 2) { return 3; }
      if (two == 3) { return 3; }
    }
    if (one == 3) {
      if (two == 0) { return 1; }
      if (two == 1) { return 2; }
      if (two == 2) { return 3; }
      if (two == 3) { return 3; }
    }
    return DEFAULT_RESULT;
}
Nick Dandoulakis
fuente
¿Por qué no tienes más?
FDinoff
3
@FDinoff Podría haber usado elsecadenas pero no había hecho ninguna diferencia.
Nick Dandoulakis
1
Sé que es trivial, pero ¿no agregaría que las cadenas else se ejecutan más rápido? en 3 de 4 casos? Siempre tengo la costumbre de escribir código para ejecutarlo lo más rápido posible, incluso si solo son unos pocos ciclos.
Brandon Bearden
2
@BrandonBearden aquí no harán ninguna diferencia (suponiendo que la entrada siempre esté en el rango 0..3). El compilador probablemente micro-optimizará el código de todos modos. Si tenemos una larga serie de else ifdeclaraciones, podemos acelerar el código con switchtablas de búsqueda.
Nick Dandoulakis
¿Cómo es eso así? Si one==0ejecutará el código, entonces tendrá que verificar si one==1entonces si one==2y finalmente si one==3- Si hubiera otros if dentro, no haría las últimas tres comprobaciones porque saldría de la declaración después de la primera coincidencia. Y sí, podría optimizar aún más mediante el uso de una declaración de cambio en lugar de las if (one...declaraciones y luego usando otro interruptor dentro del caso de la one's. Sin embargo, esa no es mi pregunta.
Brandon Bearden
6

Pruébelo con la carcasa del interruptor. ..

Echa un vistazo aquí o aquí para obtener más información al respecto.

switch (expression)
{ 
  case constant:
        statements;
        break;
  [ case constant-2:
        statements;
        break;  ] ...
  [ default:
        statements;
        break;  ] ...
}

Puede agregarle varias condiciones (no simultáneamente) e incluso tener una opción predeterminada donde no se hayan satisfecho otros casos.

PD: solo si se debe cumplir una condición ...

Si surgen 2 condiciones simultáneamente ... no creo que se pueda usar el interruptor. Pero puedes reducir tu código aquí.

Declaración de cambio de Java múltiples casos

Nevin Madhukar K
fuente
6

Lo primero que se me ocurrió fue esencialmente la misma respuesta dada por Francisco Presencia, pero optimizada de alguna manera:

public int fightMath(int one, int two)
{
    switch (one*10 + two)
    {
    case  0:
    case  1:
    case 10:
    case 11:
        return 0;
    case  2:
    case 13:
    case 21:
    case 30:
        return 1;
    case  3:
    case 12:
    case 20:
    case 31:
        return 2;
    case 22:
    case 23:
    case 32:
    case 33:
        return 3;
    }
}

Puede optimizarlo aún más haciendo que el último caso (para 3) sea el caso predeterminado:

    //case 22:
    //case 23:
    //case 32:
    //case 33:
    default:
        return 3;

La ventaja de este método es que es más fácil ver qué valores oney twoqué valores de retorno corresponden a algunos de los otros métodos sugeridos.

David R Tribble
fuente
Esta es una variación de otra respuesta mía aquí .
David R Tribble
6
((two&2)*(1+((one^two)&1))+(one&2)*(2-((one^two)&1)))/2
Dawood ibn Kareem
fuente
44
¿Cuánto tiempo te llevó llegar a esto?
mbatchkarov
2
@mbatchkarov Aproximadamente 10 minutos de leer las otras respuestas, luego 10 minutos de garabatear con lápiz y papel.
Dawood ibn Kareem el
77
Estaría muy triste si tuviera que mantener esto.
Meryovi
uhmmm ... Hay un error: te estás perdiendo; --unicorns
Alberto
Estoy de acuerdo con @Meryovi, accesorios para ser conciso, pero horrible como el código APL
Ed Griebel
3

Cuando dibujo una tabla entre uno / dos y el resultado, veo un patrón,

if(one<2 && two <2) result=0; return;

Lo anterior reduciría al menos 3 si las declaraciones. No veo un patrón establecido ni soy capaz de deducir mucho del código dado, pero si se puede derivar esa lógica, se reducirían varias declaraciones if.

Espero que esto ayude.

AnonNihcas
fuente
3

Un buen punto sería definir las reglas como texto, entonces puede derivar más fácilmente la fórmula correcta. Esto se extrae de la bonita representación de matriz de laalto:

{ 0, 0, 1, 2 },
{ 0, 0, 2, 1 },
{ 2, 1, 3, 3 },
{ 1, 2, 3, 3 }

Y aquí vamos con algunos comentarios generales, pero debe describirlos en términos de reglas:

if(one<2) // left half
{
    if(two<2) // upper left half
    {
        result = 0; //neither hits
    }
    else // lower left half
    {
        result = 1+(one+two)%2; //p2 hits if sum is even
    }
}
else // right half
{
    if(two<2) // upper right half
    {
        result = 1+(one+two+1)%2; //p1 hits if sum is even
    }
    else // lower right half
    {
        return 3; //both hit
    }
}

Por supuesto, podría reducir esto a menos código, pero generalmente es una buena idea entender lo que codifica en lugar de encontrar una solución compacta.

if((one<2)&&(two<2)) result = 0; //top left
else if((one>1)&&(two>1)) result = 3; //bottom right
else result = 1+(one+two+((one>1)?1:0))%2; //no idea what that means

Alguna explicación sobre los complicados éxitos p1 / p2 sería genial, ¡parece interesante!

Marcelo
fuente
3

La solución más corta y aún legible:

static public int fightMath(int one, int two)
{
    if (one < 2 && two < 2) return 0;
    if (one > 1 && two > 1) return 3;
    int n = (one + two) % 2;
    return one < two ? 1 + n : 2 - n;
}

o incluso más corto:

static public int fightMath(int one, int two)
{
    if (one / 2 == two / 2) return (one / 2) * 3;
    return 1 + (one + two + one / 2) % 2;
}

No contiene ningún número "mágico";) Espero que ayude.

PW
fuente
2
Las fórmulas como estas harán que sea imposible cambiar (actualizar) el resultado de una combinación en una fecha posterior. La única forma sería reelaborar toda la fórmula.
SNag
2
@SNag: estoy de acuerdo con eso. La solución más flexible es usar la matriz 2D. Pero el autor de esta publicación quería una fórmula y esta es la mejor que puede obtener con solo ifs y matemáticas.
PW
2

static int val(int i, int u){ int q = (i & 1) ^ (u & 1); return ((i >> 1) << (1 ^ q))|((u >> 1) << q); }

usuario1837841
fuente
1

Personalmente, me gusta en cascada los operadores ternarios:

int result = condition1
    ? result1
    : condition2
    ? result2
    : condition3
    ? result3
    : resultElse;

Pero en su caso, puede usar:

final int[] result = new int[/*16*/] {
    0, 0, 1, 2,
    0, 0, 2, 1,
    2, 1, 3, 3,
    1, 2, 3, 3
};

public int fightMath(int one, int two) {
    return result[one*4 + two];
}

O puede notar un patrón en bits:

one   two   result

section 1: higher bits are equals =>
both result bits are equals to that higher bits

00    00    00
00    01    00
01    00    00
01    01    00
10    10    11
10    11    11
11    10    11
11    11    11

section 2: higher bits are different =>
lower result bit is inverse of lower bit of 'two'
higher result bit is lower bit of 'two'

00    10    01
00    11    10
01    10    10
01    11    01
10    00    10
10    01    01
11    00    01
11    01    10

Entonces puedes usar magia:

int fightMath(int one, int two) {
    int b1 = one & 2, b2 = two & 2;
    if (b1 == b2)
        return b1 | (b1 >> 1);

    b1 = two & 1;

    return (b1 << 1) | (~b1);
}
Kirill Gamazkov
fuente
1

Aquí hay una versión bastante concisa, similar a la respuesta de JAB . Esto utiliza un mapa para almacenar que mueve triunfo sobre los demás.

public enum Result {
  P1Win, P2Win, BothWin, NeitherWin;
}

public enum Move {
  BLOCK_HIGH, BLOCK_LOW, ATTACK_HIGH, ATTACK_LOW;

  static final Map<Move, List<Move>> beats = new EnumMap<Move, List<Move>>(
      Move.class);

  static {
    beats.put(BLOCK_HIGH, new ArrayList<Move>());
    beats.put(BLOCK_LOW, new ArrayList<Move>());
    beats.put(ATTACK_HIGH, Arrays.asList(ATTACK_LOW, BLOCK_LOW));
    beats.put(ATTACK_LOW, Arrays.asList(ATTACK_HIGH, BLOCK_HIGH));
  }

  public static Result compare(Move p1Move, Move p2Move) {
    boolean p1Wins = beats.get(p1Move).contains(p2Move);
    boolean p2Wins = beats.get(p2Move).contains(p1Move);

    if (p1Wins) {
      return (p2Wins) ? Result.BothWin : Result.P1Win;
    }
    if (p2Wins) {
      return (p1Wins) ? Result.BothWin : Result.P2Win;
    }

    return Result.NeitherWin;
  }
} 

Ejemplo:

System.out.println(Move.compare(Move.ATTACK_HIGH, Move.BLOCK_LOW));

Huellas dactilares:

P1Win
Duncan Jones
fuente
En static final Map<Move, List<Move>> beats = new java.util.EnumMap<>();cambio, lo recomendaría , debería ser un poco más eficiente.
JAB
@JAB Sí, buena idea. Siempre olvido que ese tipo existe. Y ... ¡qué incómodo es construir uno!
Duncan Jones
1

Usaría un mapa, ya sea un HashMap o un TreeMap

Especialmente si los parámetros no están en el formulario 0 <= X < N

Como un conjunto de enteros positivos al azar.

Código

public class MyMap
{
    private TreeMap<String,Integer> map;

    public MyMap ()
    {
        map = new TreeMap<String,Integer> ();
    }

    public void put (int key1, int key2, Integer value)
    {
        String key = (key1+":"+key2);

        map.put(key, new Integer(value));
    }

    public Integer get (int key1, int key2)
    {
        String key = (key1+":"+key2);

        return map.get(key);
    }
}
Khaled.K
fuente
1

Gracias a @ Joe Harper ya que terminé usando una variación de su respuesta. Para reducirlo aún más, ya que 2 resultados por 4 fueron los mismos, lo reduje aún más.

Puedo volver a esto en algún momento, pero si no hay una gran resistencia causada por múltiples ifdeclaraciones, entonces me quedaré con esto por ahora. Examinaré la matriz de la tabla y cambiaré las soluciones de los enunciados.

public int fightMath(int one, int two) {
  if (one === 0) {
    if (two === 2) { return 1; }
    else if(two === 3) { return 2; }
    else { return 0; }
  } else if (one === 1) {
    if (two === 2) { return 2; }
    else if (two === 3) { return 1; }
    else { return 0; }
  } else if (one === 2) {
    if (two === 0) { return 2; }
    else if (two === 1) { return 1; }
    else { return 3; }
  } else if (one === 3) {
    if (two === 0) { return 1; }
    else if (two === 1) { return 2; }
    else { return 3; }
  }
}
TomFirth
fuente
13
En realidad, esto es menos legible que el original y no reduce el número de declaraciones if ...
Chad
@Chad La idea de esto era aumentar la velocidad del proceso y, aunque parece horrible, se actualiza fácilmente si agrego más acciones en el futuro. Al decir esto, ahora estoy usando una respuesta anterior que no entendía completamente antes.
TomFirth
3
@ TomFirth84 ¿Hay alguna razón por la que no sigue las convenciones de codificación adecuadas para sus declaraciones if?
ylun.ca
@ylun: había reducido las líneas antes de pegarlo en SO, no por legibilidad sino por puro espacio de spam. Hay variaciones de práctica en esta página y lamentablemente es la forma en que he aprendido y me siento cómodo.
TomFirth
2
@ TomFirth84 Yo no creo que esto se actualiza fácilmente, el número de líneas crece algo así como el producto del número de valores permitidos.
Andrew Lazarus
0
  1. Use constantes o enumeraciones para que el código sea más legible
  2. Intenta dividir el código en más funciones
  3. Intenta usar la simetría del problema

Aquí hay una sugerencia de cómo se vería esto, pero el uso de entradas aquí todavía es algo feo:

static final int BLOCK_HIGH = 0;
static final int BLOCK_LOW = 1;
static final int ATTACK_HIGH = 2;
static final int ATTACK_LOW = 3;

public static int fightMath(int one, int two) {
    boolean player1Wins = handleAttack(one, two);
    boolean player2Wins = handleAttack(two, one);
    return encodeResult(player1Wins, player2Wins); 
}



private static boolean handleAttack(int one, int two) {
     return one == ATTACK_HIGH && two != BLOCK_HIGH
        || one == ATTACK_LOW && two != BLOCK_LOW
        || one == BLOCK_HIGH && two == ATTACK_HIGH
        || one == BLOCK_LOW && two == ATTACK_LOW;

}

private static int encodeResult(boolean player1Wins, boolean player2Wins) {
    return (player1Wins ? 1 : 0) + (player2Wins ? 2 : 0);
}

Sería mejor usar un tipo estructurado para la entrada y la salida. La entrada en realidad tiene dos campos: la posición y el tipo (bloqueo o ataque). La salida también tiene dos campos: player1Wins y player2Wins. Codificar esto en un solo entero hace que sea más difícil leer el código.

class PlayerMove {
    PlayerMovePosition pos;
    PlayerMoveType type;
}

enum PlayerMovePosition {
    HIGH,LOW
}

enum PlayerMoveType {
    BLOCK,ATTACK
}

class AttackResult {
    boolean player1Wins;
    boolean player2Wins;

    public AttackResult(boolean player1Wins, boolean player2Wins) {
        this.player1Wins = player1Wins;
        this.player2Wins = player2Wins;
    }
}

AttackResult fightMath(PlayerMove a, PlayerMove b) {
    return new AttackResult(isWinningMove(a, b), isWinningMove(b, a));
}

boolean isWinningMove(PlayerMove a, PlayerMove b) {
    return a.type == PlayerMoveType.ATTACK && !successfulBlock(b, a)
            || successfulBlock(a, b);
}

boolean successfulBlock(PlayerMove a, PlayerMove b) {
    return a.type == PlayerMoveType.BLOCK 
            && b.type == PlayerMoveType.ATTACK 
            && a.pos == b.pos;
}

Desafortunadamente, Java no es muy bueno para expresar ese tipo de tipos de datos.

peq
fuente
-2

En cambio, haz algo como esto

   public int fightMath(int one, int two) {
    return Calculate(one,two)

    }


    private int Calculate(int one,int two){

    if (one==0){
        if(two==0){
     //return value}
    }else if (one==1){
   // return value as per condtiion
    }

    }
onkar
fuente
44
Acaba de crear una función privada envuelta por la pública. ¿Por qué no simplemente implementar esto en la función pública?
Martin Ueding
55
Y no ha reducido el número de declaraciones if.
Chad