Forma simple y limpia de comparar tres números

11

Tengo un código que tiene una secuencia de ifs que funciona, pero me siento desordenado. Básicamente, quiero elegir el mayor de tres enteros y establecer una bandera de estado para decir cuál fue elegido. Mi código actual se ve así:

a = countAs();
b = countBs();
c = countCs();

if (a > b && a > c)
    status = MOSTLY_A;
else if (b > a && b > c)
    status = MOSTLY_B;
else if (c > a && c > b)
    status = MOSTLY_C;
else
    status = DONT_KNOW;

Este patrón ocurre algunas veces, y con nombres largos de variables se hace un poco difícil confirmar visualmente que cada uno ifes correcto. Siento que podría haber una manera mejor y más clara de hacer esto; ¿Alguien puede sugerir algo?


Hay algunos posibles duplicados, pero no se alinean con esta pregunta.

En el duplicado sugerido: ¿ Enfoques para verificar múltiples condiciones? Todas las soluciones sugeridas parecen igualmente torpes como el código original, por lo que no proporcionan una solución mejor.

Y esta publicación Formas elegantes de manejar si (si no) más se ocupa solo de los niveles de anidación y la asimetría, que no es el problema aquí.

Ken YN
fuente
3
Para mí, el único problema es la repetición del código. Lo que tienes aquí es extremadamente claro de leer, ¿por qué cambiar eso? Simplemente divídalo en una función que tome en a, b, c y devuelva el estado. Si te hace sentir mejor, coloca un "en línea" sobre él. Sin macros, sin complejidad, solo buena extracción de funciones antiguas. Le agradeceré su código claro si necesito trabajar con él más adelante.
J Trana
Tenga en cuenta que sus #defines están mal nombrados. Considere a = 40, b = 30, c = 30. El resultado es MOSTLY_A. Pero la mayoría de las cosas en realidad no son A, sino B o C. Es posible que desee probar MORE_A (aunque todavía es ambiguo: ¿más A que B y más A que C, o más A que B y C combinados?), O A_LARGEST, MAX_IS_A, ...? (que también sugiere max (a, max (b, c)) como respuesta a la pregunta).
Tony

Respuestas:

12

Factoriza la lógica, regresa temprano

Como se sugiere en los comentarios, sería suficiente simplemente envolver su lógica en una función y salir temprano con return's' para simplificar mucho las cosas. Además, puede factorizar un poco de funcionalidad al delegar las pruebas a otra función. Más concretamente:

bool mostly(max,u,v) {
   return max > u && max > v;
}

status_t strictly_max_3(a,b,c)
{
  if mostly(a,b,c) return MOSTLY_A;
  if mostly(b,a,c) return MOSTLY_B;
  if mostly(c,a,b) return MOSTLY_C;
  return DONT_KNOW;
}

Esto es más corto que mi intento anterior:

status_t index_of_max_3(a,b,c)
{
  if (a > b) {
    if (a == c)
      return DONT_KNOW;
    if (a > c)
      return MOSTLY_A;
    else
      return MOSTLY_C;
  } else {
    if (a == b)
      return DONT_KNOW;
    if (b > c)
      return MOSTLY_B;
    else
      return MOSTLY_C;
  }
}

Lo anterior es un poco más detallado, pero es fácil de leer en mi humilde opinión y no vuelve a calcular las comparaciones varias veces.

Confirmación visual

En tu respuesta dices:

mi problema fue principalmente la confirmación visual de que todas las comparaciones usaban las mismas variables

... también, en tu pregunta, dices:

Este patrón ocurre varias veces, y con nombres de variables largos se hace un poco difícil confirmar visualmente que cada uno de ellos sea correcto.

Es posible que no entienda lo que está tratando de lograr: ¿desea copiar y pegar el patrón donde lo necesite? Con una función como la anterior, captura el patrón una vez, y verifica una vez todo lo que usan todas las comparaciones a, by csegún sea necesario. Entonces, no necesita preocuparse más cuando llama a la función. Por supuesto, tal vez en la práctica su problema sea un poco más complejo que el que describió: si es así, agregue algunos detalles si es posible.

volcado de memoria
fuente
1
No entiendo tu comentario sobre DONT_KNOW , ¿qué pasa si c es el más pequeño y a y b son iguales? El algoritmo devolvería que b es el más grande, mientras que a es lo mismo que b.
Pieter B
@PieterB estaba wronlgy suponiendo que no importa si regresamos ya sea MOSTLY_Ao MOSTLY_Cen el caso a == cy a > b. Esto está arreglado. Gracias.
coredump
@DocBrown De acuerdo, la mayoría de los beneficios provienen del comportamiento de salida anticipada.
coredump
1
+1, ahora es de hecho una mejora sobre el código original de los OP.
Doc Brown
9

TL: DR; Su código ya es correcto y "limpio".

Veo a muchas personas preguntándose por la respuesta, pero a todos les falta el bosque a través de los árboles. Hagamos la informática completa y el análisis matemático para comprender completamente esta pregunta.

Primero, notamos que tenemos 3 variables, cada una con 3 estados: <, = o>. El número total de permutaciones es 3 ^ 3 = 27 estados, a los que asignaré un número único, denotado P #, para cada estado. Este número P # es un sistema de números factoriales .

Enumerando todas las permutaciones que tenemos:

a ? b | a ? c | b ? c |P#| State
------+-------+-------+--+------------
a < b | a < c | b < c | 0| C
a = b | a < c | b < c | 1| C
a > b | a < c | b < c | 2| C
a < b | a = c | b < c | 3| impossible a<b b<a
a = b | a = c | b < c | 4| impossible a<a
a > b | a = c | b < c | 5| A=C > B
a < b | a > c | b < c | 6| impossible a<c a>c
a = b | a > c | b < c | 7| impossible a<c a>c
a > b | a > c | b < c | 8| A
a < b | a < c | b = c | 9| B=C > A
a = b | a < c | b = c |10| impossible a<a
a > b | a < c | b = c |11| impossible a<c a>c
a < b | a = c | b = c |12| impossible a<a
a = b | a = c | b = c |13| A=B=C
a > b | a = c | b = c |14| impossible a>a
a < b | a > c | b = c |15| impossible a<c a>c
a = b | a > c | b = c |16| impossible a>a
a > b | a > c | b = c |17| A
a < b | a < c | b > c |18| B
a = b | a < c | b > c |19| impossible b<c b>c
a > b | a < c | b > c |20| impossible a<c a>c
a < b | a = c | b > c |21| B
a = b | a = c | b > c |22| impossible a>a
a > b | a = c | b > c |23| impossible c>b b>c
a < b | a > c | b > c |24| B
a = b | a > c | b > c |25| A=B > C
a > b | a > c | b > c |26| A

Por inspección vemos que tenemos:

  • 3 estados donde A es el máximo,
  • 3 estados donde B es el máximo,
  • 3 estados donde C es el máximo, y
  • 4 estados donde A = B o B = C.

Escribamos un programa (vea la nota al pie) para enumerar todas estas permutaciones con valores para A, B y C. Clasificación estable por P #:

a ?? b | a ?? c | b ?? c |P#| State
1 <  2 | 1 <  3 | 2 <  3 | 0| C
1 == 1 | 1 <  2 | 1 <  2 | 1| C
1 == 1 | 1 <  3 | 1 <  3 | 1| C
2 == 2 | 2 <  3 | 2 <  3 | 1| C
2  > 1 | 2 <  3 | 1 <  3 | 2| C
2  > 1 | 2 == 2 | 1 <  2 | 5| ??
3  > 1 | 3 == 3 | 1 <  3 | 5| ??
3  > 2 | 3 == 3 | 2 <  3 | 5| ??
3  > 1 | 3  > 2 | 1 <  2 | 8| A
1 <  2 | 1 <  2 | 2 == 2 | 9| ??
1 <  3 | 1 <  3 | 3 == 3 | 9| ??
2 <  3 | 2 <  3 | 3 == 3 | 9| ??
1 == 1 | 1 == 1 | 1 == 1 |13| ??
2 == 2 | 2 == 2 | 2 == 2 |13| ??
3 == 3 | 3 == 3 | 3 == 3 |13| ??
2  > 1 | 2  > 1 | 1 == 1 |17| A
3  > 1 | 3  > 1 | 1 == 1 |17| A
3  > 2 | 3  > 2 | 2 == 2 |17| A
1 <  3 | 1 <  2 | 3  > 2 |18| B
1 <  2 | 1 == 1 | 2  > 1 |21| B
1 <  3 | 1 == 1 | 3  > 1 |21| B
2 <  3 | 2 == 2 | 3  > 2 |21| B
2 <  3 | 2  > 1 | 3  > 1 |24| B
2 == 2 | 2  > 1 | 2  > 1 |25| ??
3 == 3 | 3  > 1 | 3  > 1 |25| ??
3 == 3 | 3  > 2 | 3  > 2 |25| ??
3  > 2 | 3  > 1 | 2  > 1 |26| A

En caso de que te estés preguntando cómo sabía qué estados P # eran imposibles, ahora lo sabes. :-)

El número mínimo de comparaciones para determinar el orden es:

Log2 (27) = Log (27) / Log (2) = ~ 4.75 = 5 comparaciones

es decir, coredump dio el número mínimo correcto de 5 comparaciones. Daría formato a su código como:

status_t index_of_max_3(a,b,c)
{
    if (a > b) {
        if (a == c) return DONT_KNOW; // max a or c
        if (a >  c) return MOSTLY_A ;
        else        return MOSTLY_C ;
    } else {
        if (a == b) return DONT_KNOW; // max a or b
        if (b >  c) return MOSTLY_B ;
        else        return MOSTLY_C ;
    }
}

Para su problema, no nos interesan las pruebas de igualdad, por lo que podemos omitir 2 pruebas.

¡No importa cuán limpio / malo sea el código si obtiene la respuesta incorrecta, por lo que es una buena señal de que está manejando todos los casos correctamente!

Luego, en lo que respecta a la simplicidad, las personas siguen tratando de "mejorar" la respuesta, donde piensan que mejorar significa "optimizar" el número de comparaciones, pero eso no es estrictamente lo que usted pregunta. Confundiste a todos cuando preguntaste "Siento que podría haber un mejor" pero no definiste qué significa "mejor". Menos comparaciones? ¿Menos código? ¿Comparaciones óptimas?

Ahora, ya que está preguntando sobre la legibilidad del código (dada la corrección), solo haría un cambio en su código para la legibilidad: alinee la primera prueba con las otras.

        if      (a > b && a > c)
            status = MOSTLY_A;
        else if (b > a && b > c)
            status = MOSTLY_B;
        else if (c > a && c > b)
            status = MOSTLY_C;
        else
            status = DONT_KNOW; // a=b or b=c, we don't care

Personalmente, lo escribiría de la siguiente manera, pero esto puede ser demasiado poco ortodoxo para sus estándares de codificación:

        if      (a > b && a > c) status = MOSTLY_A ;
        else if (b > a && b > c) status = MOSTLY_B ;
        else if (c > a && c > b) status = MOSTLY_C ;
        else /*  a==b  || b ==c*/status = DONT_KNOW; // a=b or b=c, we don't care

Nota al pie: Aquí está el código C ++ para generar las permutaciones:

#include <stdio.h>

char txt[]  = "< == > ";
enum cmp      { LESS, EQUAL, GREATER };
int  val[3] = { 1, 2, 3 };

enum state    { DONT_KNOW, MOSTLY_A, MOSTLY_B, MOSTLY_C };
char descr[]= "??A B C ";

cmp Compare( int x, int y ) {
    if( x < y ) return LESS;
    if( x > y ) return GREATER;
    /*  x==y */ return EQUAL;
}

int main() {
    int i, j, k;
    int a, b, c;

    printf( "a ?? b | a ?? c | b ?? c |P#| State\n" );
    for( i = 0; i < 3; i++ ) {
        a = val[ i ];
        for( j = 0; j < 3; j++ ) {
            b = val[ j ];
            for( k = 0; k < 3; k++ ) {
                c = val[ k ];

                int cmpAB = Compare( a, b );
                int cmpAC = Compare( a, c );
                int cmpBC = Compare( b, c );
                int n     = (cmpBC * 9) + (cmpAC * 3) + cmpAB; // Reconstruct unique P#

                printf( "%d %c%c %d | %d %c%c %d | %d %c%c %d |%2d| "
                    , a, txt[cmpAB*2+0], txt[cmpAB*2+1], b
                    , a, txt[cmpAC*2+0], txt[cmpAC*2+1], c
                    , b, txt[cmpBC*2+0], txt[cmpBC*2+1], c
                    , n
                );

                int status;
                if      (a > b && a > c) status = MOSTLY_A;
                else if (b > a && b > c) status = MOSTLY_B;
                else if (c > a && c > b) status = MOSTLY_C;
                else /*  a ==b || b== c*/status = DONT_KNOW; // a=b, or b=c

                printf( "%c%c\n", descr[status*2+0], descr[status*2+1] );
            }
        }
    }
    return 0;
}

Ediciones: Basado en comentarios, movió TL: DR a la parte superior, eliminó la tabla sin clasificar, aclaró 27, limpió el código, describió los estados imposibles.

Michaelangel007
fuente
-1: ¿no reduciría el número de decisiones a rutas de código más simples y a un código más legible? Tu argumento no es claro: primero, dices que todos están equivocados; entonces, pones no una o dos sino tres tablas; Esperaba que condujeran a una forma más simple de calcular el resultado, pero en lugar de eso confirmó lo que todos los demás ya sabían (el código de OP hace lo correcto). Claro, la pregunta es acerca de la legibilidad, pero la legibilidad no se logra solo modificando el diseño del código (admite que sus cambios difícilmente se ajustarían a los estándares de código existentes). Tiene sentido simplificar la lógica al optimizar la legibilidad.
coredump
Más constructivamente: sugeriría simplificar su respuesta omitiendo algunos detalles y pensando en la estructura de su respuesta. Aprecio que se haya tomado el tiempo de escribir y publicar el código C ++ generando permutaciones, pero tal vez podría dar el resultado principal y solo una tabla: como está ahora, parece que dejó todo su trabajo tal como está. Casi no pude detectar la cosa TL; DR (podría comenzar con eso). Espero eso ayude.
coredump
2
Gracias por el feedback constructivo coredump. Eliminé la tabla sin clasificar del medio ya que se verifica fácilmente.
Michaelangel007
2
¡Jesucristo! ¿Quién diría que comparar tres números es casi tan complejo como la ciencia espacial?
Mandrill
@Mandrill Como informáticos, nuestro trabajo es comprender a fondo un problema . Solo enumerando las 27 permutaciones posibles para una comparación de 3 vías podemos verificar que nuestra solución funciona en TODOS los casos. Lo último que queremos como programadores son errores ocultos y casos extremos no contabilizados. La verbosidad es el precio que se paga por la corrección.
Michaelangel007
5

@msw le dijo que usara una matriz en lugar de a, b, c, y @Basile le dijo que refactorizara la lógica "max" en una función. La combinación de estas dos ideas conduce a

val[0] = countAs();    // in the real code, one should probably define 
val[1] = countBs();    // some enum for the indexes 0,1,2 here
val[2] = countCs();

 int result[]={DONT_KNOW, MOSTLY_A, MOSTLY_B, MOSTLY_C};

luego proporcione una función que calcule el índice máximo de una matriz arbitraria:

// returns the index of the strict maximum, and -1 when the maximum is not strict
int FindStrictMaxIndex(int *values,int arraysize)
{
    int maxVal=INT_MIN;
    int maxIndex=-1;
    for(int i=0;i<arraysize;++i)
    {
       if(values[i]>maxVal)
       {
         maxVal=values[i];
         maxIndex=i;
       }
       else if (values[i]==maxVal)
       {
         maxIndex=-1;
       }
    }
    return maxIndex;
}

y llámalo como

 return result[FindStrictMaxIndex(val,3)+1];

El número total de LOC parece haber aumentado sobre el original, pero ahora tiene la lógica central en una función reutilizable, y si puede reutilizar la función varias veces, comienza a dar sus frutos. Además, la FindStrictMaxIndexfunción ya no está entretejida con sus "requisitos comerciales" (separación de preocupaciones), por lo tanto, el riesgo que tendrá que modificar más adelante es mucho menor que en su versión original (principio abierto-cerrado). Por ejemplo, esa función no tendrá que cambiarse incluso si cambia el número de argumentos, o si necesita usar otros valores de retorno que MOSTLY_ABC, o si está procesando otras variables que no sean a, b, c. Además, el uso de una matriz en lugar de 3 valores diferentes a, b, c también podría simplificar su código en otros lugares.

Por supuesto, si en todo su programa solo hay uno o dos lugares para llamar a esta función, y no tiene más aplicaciones para mantener los valores en una matriz, entonces probablemente dejaría el código original como está (o use @ mejora de coredump).

Doc Brown
fuente
Me gusta: las tripas FindStrictMaxIndex()pueden no estar demasiado limpias, pero desde el punto de vista de la persona que llama es bastante obvio lo que se está tratando de lograr.
Ken YN
O en lugar de mantener dos matrices, mantenga una matriz de pares clave-valor: {MOSTLY_A, countAs ()}, tome el primer elemento ordenado por valor y lea la clave.
Julia Hayward
@JuliaHayward: la razón principal por la que no sugerí tal solución fue la etiqueta "C" de la pregunta: en C, uno necesitará más código repetitivo para lidiar con pares clave-valor y crear una función escrita en términos de KVP probablemente no sea tan reutilizable en diferentes contextos que una simple intfunción escrita. Pero acepto su comentario si uno usa un lenguaje diferente como Python o Perl.
Doc Brown
1
@ gnasher729: depende de cuántos "duplicados" en el código original son, qué tan similares son realmente y con qué frecuencia FindStrictMaxIndexse puede reutilizar la función. Por una o solo dos veces de reutilización, esto no valdrá la pena, por supuesto, pero eso es lo que ya escribí en mi respuesta. Tenga en cuenta también las otras ventajas que mencioné anteriormente con respecto a los cambios futuros.
Doc Brown
1
... y tenga en cuenta que las 8 líneas originales se pueden reemplazar por una línea simple return result[FindStrictMaxIndex(val,3)]; en el punto del código donde se colocaron las 8 líneas originales . Las otras partes, especialmente en FindStrictMaxIndexsí mismas, están completamente separadas de la "lógica empresarial", que las saca del foco de los requisitos comerciales cambiantes.
Doc Brown
-1

Probablemente deberías usar una macro o una función que MAX dé el máximo de dos números.

Entonces solo quieres:

 status = MAX(a,MAX(b,c));

Es posible que hayas definido

 #define MAX(X,Y) (((X)>(Y))?(X):(Y))

pero tenga cuidado, notablemente sobre los efectos secundarios, cuando use macros (ya que MAX(i++,j--) se comportaría de manera extraña)

Así que mejor define una función

 static inline int max2ints(int x, int y) {
    return (x>y)?x:y;
 }

y úsalo (o al menos #define MAX(X,Y) max2ints((X),(Y))...)

Si necesita comprender el origen del MAX, es posible que tenga una macro larga como la #define COMPUTE_MAX_WITH_CAUSE(Status,Result,X,Y,Z) que es larga do{... }while(0) macro, tal vez

#define COMPUTE_MAX_WITH_CAUSE(Status,Result,X,Y,Z) do { \
  int x= (X), y= (Y), z=(Z); \
  if (x > y && y > z) \
    { Status = MOSTLY_FIRST; Result = x; } \
  else if (y > x && y > z) \
    { Status = MOSTLY_SECOND; Result = y; } \
  else if (z > x && z > y) \
    { Status = MOSTLY_THIRD; Result = z; } \
  /* etc */ \
  else { Status = UNKNOWN; Result = ... } \
} while(0)

Entonces podrías invocar COMPUTE_MAX_WITH_CAUSE(status,res,a,b,c) en varios lugares. Es un poco feo. He definido las variables locales x, y, z para reducir los efectos secundarios malos ....

Basile Starynkevitch
fuente
2
Refactorizar la lógica común en una función es el enfoque correcto, pero en realidad evitaría dos cosas aquí: 1. No "inventaría" nuevos requisitos (el OP no solicitó calcular el máximo). Y segundo: incluso si el código resultante puede volverse más SECO, es muy discutible si esto justifica una macro complicada.
Doc Brown
1
Las macros deberían ser una herramienta de último recurso. Definitivamente fuera de límites para este problema.
Kevin Cline
-1

He pensado más sobre esto, así que dado que mi problema era principalmente la confirmación visual de que todas las comparaciones usaban las mismas variables, creo que este podría ser un enfoque útil:

a = countAs();
b = countBs();
c = countCs();

if (FIRST_IS_LARGEST(a, b, c))
    status = MOSTLY_A;
else if (SECOND_IS_LARGEST(a, b, c))
    status = MOSTLY_B;
else if (THIRD_IS_LARGEST(a, b, c))
    status = MOSTLY_C;
else
    status = DONT_KNOW; /* NO_SINGLE_LARGEST is a better name? */

Que cada macro toma a, by cen el mismo orden es fácil de confirmar, y el nombre de la macro me ahorra tener que averiguar qué están haciendo todas las comparaciones y AND.

Ken YN
fuente
1
(1) ¿por qué macros auxiliares en lugar de funciones? (2) ¿por qué necesita confirmación visual aquí? ¿Es realmente su problema central o es la necesidad de confirmación visual una consecuencia de la duplicación de código? Su mejor opción es factorizar su código en una única función simple que verifique de una vez por todas .
coredump