Einfache und saubere Methode zum Vergleichen von drei Zahlen

Ich habe einen Code mit einer Folge von ifs, die funktionieren, aber ich fühle mich einfach chaotisch. Grundsätzlich möchte ich die größte von drei Ganzzahlen auswählen und ein Statusflag setzen, um zu sagen, welche ausgewählt wurde. Mein aktueller Code sieht folgendermaßen aus:

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;

Dieses Muster tritt einige Male auf, und bei langen Variablennamen wird es etwas schwierig, visuell zu bestätigen, dass jedes ifkorrekt ist. Ich denke, es könnte einen besseren und klareren Weg geben, dies zu tun. kann jemand etwas vorschlagen?

Es gibt einige potenzielle Duplikate, die jedoch nicht ganz mit dieser Frage übereinstimmen.

Im vorgeschlagenen Duplikat: Ansätze zur Überprüfung mehrerer Bedingungen? Alle vorgeschlagenen Lösungen scheinen genauso ungeschickt zu sein wie der ursprüngliche Code, daher bieten sie keine bessere Lösung.

Und dieser Beitrag Elegante Möglichkeiten, wenn (falls sonst) sonst nur Verschachtelungsebenen und Asymmetrie behandelt werden, was hier nicht das Problem ist.

Logik faktorisieren, früh zurückkehren

Wie in den Kommentaren vorgeschlagen, würde es ausreichen, Ihre Logik einfach in eine Funktion einzuschließen und frühzeitig mit return's zu beenden, um die Dinge erheblich zu vereinfachen. Sie können auch ein wenig Funktionalität berücksichtigen, indem Sie Tests an eine andere Funktion delegieren. Konkreter:

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

Dies ist kürzer als mein vorheriger Versuch:

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

Das Obige ist etwas ausführlicher, aber meiner Meinung nach leicht zu lesen und berechnet Vergleiche nicht mehrmals neu.

Visuelle Bestätigung

In Ihrer Antwort sagen Sie:

Mein Problem war hauptsächlich die visuelle Bestätigung, dass alle Vergleiche dieselben Variablen verwendeten

... auch in Ihrer Frage sagen Sie:

Dieses Muster tritt einige Male auf, und bei langen Variablennamen wird es etwas schwierig, visuell zu bestätigen, dass jedes if korrekt ist.

Ich verstehe möglicherweise nicht, was Sie erreichen möchten: Möchten Sie das Muster überall dort kopieren und einfügen, wo Sie es benötigen? Mit einer Funktion wie der obigen erfassen Sie das Muster einmal und überprüfen einmal, ob alle Vergleiche verwendet werden a, bund cnach Bedarf. Dann brauchen Sie sich keine Sorgen mehr zu machen, wenn Sie die Funktion aufrufen. Natürlich ist Ihr Problem in der Praxis vielleicht etwas komplexer als das von Ihnen beschriebene: Wenn ja, fügen Sie bitte nach Möglichkeit einige Details hinzu.

TL: DR; Ihr Code ist bereits korrekt und "sauber".

Ich sehe viele Leute, die um die Antwort waffeln, aber jeder vermisst den Wald durch die Bäume. Lassen Sie uns die gesamte Informatik und mathematische Analyse durchführen, um diese Frage vollständig zu verstehen.

Zunächst stellen wir fest, dass wir 3 Variablen mit jeweils 3 Zuständen haben: <, = oder>. Die Gesamtzahl der Permutationen beträgt 3 ^ 3 = 27 Zustände, wobei ich jedem Zustand eine eindeutige Zahl mit der Bezeichnung P # zuweisen werde. Diese P # -Nummer ist ein Fakultätsnummernsystem .

Aufzählung aller Permutationen, die wir haben:

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

Durch Inspektion sehen wir, dass wir haben:

• 3 Zustände, in denen A das Maximum ist,
• 3 Zustände, in denen B das Maximum ist,
• 3 Zustände, in denen C das Maximum ist, und
• 4 Zustände, in denen entweder A = B oder B = C ist.

Schreiben wir ein Programm (siehe Fußnote), um alle diese Permutationen mit Werten für A, B und C aufzulisten. Stabile Sortierung nach 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

Falls Sie sich gefragt haben, woher ich weiß, welche P # -Zustände unmöglich sind, wissen Sie jetzt. :-)

Die Mindestanzahl von Vergleichen zur Bestimmung der Reihenfolge beträgt:

Log2 (27) = Log (27) / Log (2) = ~ 4,75 = 5 Vergleiche

dh Coredump ergab die korrekte 5 minimale Anzahl von Vergleichen. Ich würde seinen Code wie folgt formatieren:

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

Für Ihr Problem ist es uns egal, ob wir auf Gleichheit prüfen, daher können wir zwei Tests weglassen.

Es spielt keine Rolle, wie sauber / schlecht der Code ist, wenn er die falsche Antwort erhält. Dies ist ein gutes Zeichen dafür, dass Sie alle Fälle korrekt behandeln!

Der Einfachheit halber versuchen die Leute immer wieder, die Antwort zu "verbessern", wobei sie denken, dass Verbesserung bedeutet, die Anzahl der Vergleiche zu "optimieren", aber das ist nicht genau das, was Sie fragen. Sie haben alle verwirrt, bei denen Sie gefragt haben: "Ich glaube, es könnte ein besseres geben", aber nicht definiert, was "besser" bedeutet. Weniger Vergleiche? Weniger Code? Optimale Vergleiche?

Da Sie nun nach der Lesbarkeit des Codes fragen (bei korrekter Richtigkeit), würde ich nur eine Änderung an Ihrem Code vornehmen, um die Lesbarkeit zu gewährleisten: Richten Sie den ersten Test an den anderen aus.

        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

Persönlich würde ich es folgendermaßen schreiben, aber dies ist möglicherweise zu unorthodox für Ihre Codierungsstandards:

        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

Fußnote: Hier ist der C ++ - Code zum Generieren der Permutationen:

#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;
}

Bearbeitungen: Basierend auf Feedback, TL: DR nach oben verschoben, unsortierte Tabelle entfernt, 27 geklärt, Code bereinigt, unmögliche Zustände beschrieben.

@msw hat Ihnen gesagt, dass Sie ein Array anstelle von a, b, c und verwenden sollen. @Basile hat Ihnen gesagt, dass Sie die "max" -Logik in eine Funktion umgestalten sollen. Die Kombination dieser beiden Ideen führt zu

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

Stellen Sie dann eine Funktion bereit, die den maximalen Index eines beliebigen Arrays berechnet:

// 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;
}

und nenne es wie

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

Die Gesamtzahl der LOC scheint sich gegenüber der ursprünglichen erhöht zu haben, aber jetzt haben Sie die Kernlogik in einer wiederverwendbaren Funktion, und wenn Sie die Funktion mehrmals wiederverwenden können, beginnt sie sich auszuzahlen. Darüber hinaus ist die FindStrictMaxIndexFunktion nicht mehr mit Ihren "Geschäftsanforderungen" verwoben (Trennung von Bedenken), sodass das Risiko, dass Sie sie später ändern müssen, viel geringer ist als in Ihrer ursprünglichen Version (Open-Closed-Prinzip). Diese Funktion muss beispielsweise nicht geändert werden, auch wenn sich die Anzahl der Argumente ändert oder andere Rückgabewerte als MOSTLY_ABC verwendet werden müssen oder Sie andere Variablen als a, b, c verarbeiten. Darüber hinaus kann die Verwendung eines Arrays anstelle von 3 verschiedenen Werten a, b, c Ihren Code auch an anderen Stellen vereinfachen.

Wenn es in Ihrem gesamten Programm nur ein oder zwei Stellen gibt, an denen diese Funktion aufgerufen werden kann, und Sie keine weiteren Anwendungen zum Speichern der Werte in einem Array haben, würde ich den ursprünglichen Code wahrscheinlich so lassen, wie er ist (oder verwendet wird) @ coredumps Verbesserung).

Sie sollten wahrscheinlich ein Makro oder eine Funktion verwenden MAX , die maximal zwei Zahlen enthält.

Dann wollen Sie einfach:

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

Sie könnten definiert haben

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

Seien Sie jedoch vorsichtig - insbesondere in Bezug auf Nebenwirkungen -, wenn Sie Makros verwenden (da MAX(i++,j--) sich dies seltsam verhalten würde).

Definieren Sie also besser eine Funktion

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

und benutze es (oder zumindest #define MAX(X,Y) max2ints((X),(Y))....)

Wenn Sie den Ursprung des MAX verstehen müssen, haben Sie möglicherweise ein langes Makro, wie #define COMPUTE_MAX_WITH_CAUSE(Status,Result,X,Y,Z) es vielleicht ein langes do{... }while(0)Makro ist

#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)

Dann könnten Sie COMPUTE_MAX_WITH_CAUSE(status,res,a,b,c) an mehreren Stellen aufrufen . Es ist ein bisschen hässlich. Ich definierte lokale Variablen x, y, z schlechte Nebenwirkungen zu senken ....

Ich habe mehr darüber nachgedacht. Da mein Problem hauptsächlich die visuelle Bestätigung war, dass alle Vergleiche dieselben Variablen verwendeten, denke ich, dass dies ein nützlicher Ansatz sein könnte:

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? */

Dass jeder Makro nimmt a, bund cin der gleichen Reihenfolge , ist einfach zu bestätigen, und der Makroname spart mich mit, herauszufinden , was alle Vergleiche und ANDs tun.