Cara sederhana dan bersih membandingkan tiga angka

Saya punya beberapa kode yang memiliki urutan ifs yang berfungsi, tetapi hanya merasa berantakan. Pada dasarnya, saya ingin memilih yang terbesar dari tiga bilangan bulat dan menetapkan bendera status untuk mengatakan yang dipilih. Kode saya saat ini terlihat seperti ini:

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;

Pola ini muncul beberapa kali, dan dengan nama variabel yang panjang akan sedikit sulit untuk memastikan secara visual bahwa masing-masing ifsudah benar. Saya merasa mungkin ada cara yang lebih baik dan lebih jelas untuk melakukan ini; adakah yang bisa menyarankan sesuatu?

Ada beberapa duplikat potensial, tetapi mereka tidak cukup sejalan dengan pertanyaan ini.

Dalam duplikat yang disarankan: Pendekatan untuk memeriksa berbagai kondisi? semua solusi yang disarankan tampak sama kikuknya dengan kode asli, sehingga tidak memberikan solusi yang lebih baik.

Dan posting ini cara-cara yang elegan untuk menangani jika (jika lain) hanya berurusan dengan tingkat dan asimetri bersarang, yang tidak menjadi masalah di sini.

Memfaktorkan logika, kembali lebih awal

Seperti yang disarankan dalam komentar, itu akan cukup untuk hanya membungkus logika Anda dalam suatu fungsi dan keluar lebih awal dengan returnitu untuk menyederhanakan banyak hal. Juga, Anda bisa memfaktorkan sedikit fungsionalitas dengan mendelegasikan tes ke fungsi lain. Lebih konkret:

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

Ini lebih pendek dari upaya saya sebelumnya:

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

Di atas sedikit lebih verbose, tetapi mudah dibaca IMHO dan tidak menghitung ulang perbandingan beberapa kali.

Konfirmasi visual

Dalam jawaban Anda, Anda mengatakan:

masalah saya sebagian besar konfirmasi visual bahwa semua perbandingan menggunakan variabel yang sama

... juga, dalam pertanyaan Anda, Anda mengatakan:

Pola ini terjadi beberapa kali, dan dengan nama variabel yang panjang akan sedikit sulit untuk mengkonfirmasi secara visual bahwa masing-masing jika benar.

Saya mungkin tidak mengerti apa yang ingin Anda capai: apakah Anda ingin menyalin-menempel pola di mana pun Anda membutuhkannya? Dengan fungsi seperti yang di atas, Anda menangkap pola sekali, dan Anda memeriksa sekali untuk semua yang digunakan semua perbandingan a, bdan csesuai kebutuhan. Maka, Anda tidak perlu khawatir lagi saat memanggil fungsi. Tentu saja, mungkin dalam praktiknya masalah Anda sedikit lebih kompleks daripada yang Anda jelaskan: jika demikian, tambahkan beberapa detail jika mungkin.

TL: DR; Kode Anda sudah benar dan "bersih".

Saya melihat banyak orang bertanya-tanya tetapi semua orang kehilangan hutan melalui pepohonan. Mari kita lakukan ilmu komputer penuh dan analisis matematika untuk sepenuhnya memahami pertanyaan ini.

Pertama, kita perhatikan bahwa kita memiliki 3 variabel, masing-masing dengan 3 menyatakan: <, =, atau>. Jumlah total permutasi adalah 3 ^ 3 = 27 status, yang akan saya berikan nomor unik, dilambangkan dengan P #, untuk setiap status. Nomor P # ini adalah sistem angka faktorial .

Merinci semua permutasi yang kita miliki:

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

Dengan inspeksi kami melihat kami memiliki:

• 3 menyatakan di mana A adalah maks,
• 3 menyatakan di mana B adalah maks,
• 3 menyatakan di mana C adalah maks, dan
• 4 menyatakan di mana A = B, atau B = C.

Mari kita menulis sebuah program (lihat catatan kaki) untuk menghitung semua permutasi ini dengan nilai untuk A, B, dan C. Penyortiran yang stabil dengan 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

Jika Anda bertanya-tanya bagaimana saya tahu negara bagian mana yang tidak mungkin, sekarang Anda tahu. :-)

Jumlah perbandingan minimum untuk menentukan pesanan adalah:

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

yaitu coredump memberikan 5 perbandingan jumlah minimal yang benar. Saya akan memformat kodenya sebagai:

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

Untuk masalah Anda, kami tidak peduli tentang pengujian untuk kesetaraan sehingga kami dapat menghilangkan 2 tes.

Tidak masalah seberapa bersih / buruk kodenya jika mendapat jawaban yang salah jadi ini pertanda baik bahwa Anda menangani semua kasing dengan benar!

Selanjutnya, untuk kesederhanaan, orang-orang terus berusaha untuk "meningkatkan" jawabannya, di mana mereka berpikir meningkatkan berarti "mengoptimalkan" jumlah perbandingan, tetapi itu tidak sepenuhnya apa yang Anda minta. Anda bingung semua orang di mana Anda bertanya "Saya merasa mungkin ada yang lebih baik" tetapi tidak mendefinisikan apa artinya 'lebih baik'. Lebih sedikit perbandingan? Kode kurang? Perbandingan yang optimal?

Sekarang karena Anda bertanya tentang keterbacaan kode (diberikan kebenaran) saya hanya akan membuat satu perubahan pada kode Anda untuk keterbacaan: Sejajarkan tes pertama dengan yang lain.

        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

Secara pribadi saya akan menulisnya dengan cara berikut tetapi ini mungkin terlalu tidak lazim untuk standar pengkodean Anda:

        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

Catatan Kaki: Berikut ini adalah kode C ++ untuk menghasilkan permutasi:

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

Suntingan: Berdasarkan umpan balik, memindahkan TL: DR ke atas, menghapus tabel yang tidak disortir, mengklarifikasi 27, membersihkan kode, menggambarkan keadaan yang mustahil.

@ msw memberi tahu Anda untuk menggunakan array alih-alih a, b, c, dan @Basile memberi tahu Anda refactor logika "maks" menjadi fungsi. Menggabungkan dua gagasan ini mengarah ke

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

lalu berikan fungsi yang menghitung indeks maks dari array arbitrer:

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

dan menyebutnya seperti

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

Jumlah total LOC tampaknya telah meningkat dari yang asli, tetapi sekarang Anda memiliki logika inti dalam fungsi yang dapat digunakan kembali, dan jika Anda dapat menggunakan kembali fungsi beberapa kali, itu mulai membuahkan hasil. Selain itu, FindStrictMaxIndexfungsi tersebut tidak terjalin dengan "persyaratan bisnis" Anda lagi (pemisahan kekhawatiran), sehingga risiko Anda harus memodifikasinya nanti jauh lebih rendah daripada di versi asli Anda (prinsip buka-tutup). Misalnya, fungsi itu tidak perlu diubah bahkan jika jumlah argumen berubah, atau perlu menggunakan nilai balik lain selain MOSTLY_ABC, atau Anda sedang memproses variabel lain selain a, b, c. Selain itu, penggunaan array bukan 3 nilai berbeda a, b, c mungkin menyederhanakan kode Anda di tempat lain juga.

Tentu saja, jika di seluruh program Anda hanya ada satu atau dua tempat untuk memanggil fungsi ini, dan Anda tidak memiliki aplikasi lebih lanjut untuk menyimpan nilai-nilai dalam array, maka saya mungkin akan meninggalkan kode asli seperti itu (atau menggunakan @ peningkatan coredump).

Anda mungkin harus menggunakan makro atau fungsi yang MAX memberikan maksimal dua angka.

Maka Anda hanya ingin:

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

Anda mungkin telah mendefinisikan

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

tapi hati-hati - terutama tentang efek samping - saat menggunakan makro (karena MAX(i++,j--) akan berperilaku aneh)

Jadi lebih baik mendefinisikan suatu fungsi

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

dan gunakan (atau setidaknya #define MAX(X,Y) max2ints((X),(Y))....)

Jika Anda perlu memahami asal-usul MAX Anda mungkin memiliki makro panjang seperti #define COMPUTE_MAX_WITH_CAUSE(Status,Result,X,Y,Z) yang merupakan do{... }while(0) makro panjang , mungkin

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

Maka Anda bisa memohon COMPUTE_MAX_WITH_CAUSE(status,res,a,b,c) di beberapa tempat. Agak jelek. Saya mendefinisikan variabel lokal x, y, z untuk menurunkan efek samping yang buruk ....

Saya sudah lebih banyak memikirkan hal ini, jadi karena masalah saya kebanyakan adalah konfirmasi visual bahwa semua perbandingan menggunakan variabel yang sama, saya pikir ini mungkin pendekatan yang berguna:

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

Bahwa setiap makro mengambil a, bdan cdalam urutan yang sama mudah untuk dikonfirmasi, dan nama makro menyelamatkan saya harus mencari tahu apa yang dilakukan semua perbandingan dan AND.