Bagaimana cara saya mengedit rantai pernyataan if-else jika mematuhi prinsip-prinsip Kode Paman Bob?

Saya mencoba mengikuti saran kode bersih Paman Bob dan secara khusus membuat metode singkat.

Saya menemukan diri saya tidak dapat mempersingkat logika ini:

if (checkCondition()) {addAlert(1);}
else if (checkCondition2()) {addAlert(2);}
else if (checkCondition3()) {addAlert(3);}
else if (checkCondition4()) {addAlert(4);}

Saya tidak dapat menghapus elses dan dengan demikian memisahkan semuanya menjadi bit yang lebih kecil, menyebabkan "lain" di "lain jika" membantu kinerja - mengevaluasi kondisi tersebut mahal dan jika saya dapat menghindari mengevaluasi kondisi di bawah ini, menyebabkan salah satu yang pertama itu benar, saya ingin menghindarinya.

Bahkan secara semantik, mengevaluasi kondisi berikutnya jika sebelumnya dipenuhi tidak masuk akal dari sudut pandang bisnis.

sunting: Pertanyaan ini diidentifikasi sebagai duplikat dari cara-cara yang elegan untuk menangani if ​​(if else) .

Saya percaya ini adalah pertanyaan yang berbeda (Anda dapat melihatnya juga dengan membandingkan jawaban dari pertanyaan-pertanyaan itu).

• Pertanyaan saya adalah memeriksa kondisi penerimaan pertama berakhir dengan cepat .
• Pertanyaan yang ditautkan adalah mencoba agar semua persyaratan dapat diterima untuk melakukan sesuatu. (lebih baik dilihat dalam jawaban untuk pertanyaan itu: https://softwareengineering.stackexchange.com/a/122625/96955 )

Idealnya saya pikir Anda harus mengekstrak logika Anda untuk mendapatkan kode / nomor peringatan ke dalam metode sendiri. Jadi kode Anda yang ada berkurang hingga

{
    addAlert(GetConditionCode());
}

dan Anda memiliki GetConditionCode () merangkum logika untuk memeriksa kondisi. Mungkin juga lebih baik menggunakan Enum daripada angka ajaib.

private AlertCode GetConditionCode() {
    if (CheckCondition1()) return AlertCode.OnFire;
    if (CheckCondition2()) return AlertCode.PlagueOfBees;
    if (CheckCondition3()) return AlertCode.Godzilla;
    if (CheckCondition4()) return AlertCode.ZombieSharkNado;
    return AlertCode.None;
}

Pengukuran penting adalah kompleksitas kode, bukan ukuran absolut. Dengan asumsi bahwa kondisi yang berbeda benar-benar hanya panggilan fungsi tunggal, sama seperti tindakan tidak lebih kompleks dari apa yang Anda tunjukkan, saya akan mengatakan tidak ada yang salah dengan kode. Ini sudah sesederhana mungkin.

Setiap upaya untuk "menyederhanakan" lebih lanjut memang akan memperumit masalah.

Tentu saja, Anda dapat mengganti elsekata kunci dengan returnseperti yang disarankan orang lain, tetapi itu hanya masalah gaya, bukan perubahan dalam kompleksitas apa pun.

Ke samping:

Saran umum saya adalah, jangan pernah menjadi religius tentang aturan apa pun untuk kode bersih: Sebagian besar saran pengkodean yang Anda lihat di internet baik jika diterapkan dalam konteks yang sesuai, tetapi menerapkan secara radikal bahwa saran yang sama di mana-mana dapat membuat Anda mendapatkan entri di yang IOCCC . Caranya adalah selalu untuk mencapai keseimbangan yang memungkinkan manusia untuk dengan mudah beralasan tentang kode Anda.

Gunakan metode yang terlalu besar, dan Anda kacau. Gunakan fungsi terlalu kecil, dan Anda kacau. Hindari ekspresi ternary, dan Anda kacau. Gunakan ekspresi ternary di mana-mana, dan Anda kacau. Sadarilah bahwa ada tempat-tempat yang memanggil fungsi-fungsi satu jalur, dan tempat-tempat yang memanggil fungsi 50-saluran (ya, mereka ada!). Sadarilah bahwa ada tempat yang membutuhkan if()pernyataan, dan ada tempat yang meminta ?:operator. Gunakan gudang senjata lengkap yang Anda inginkan, dan cobalah untuk selalu menggunakan alat paling pas yang bisa Anda temukan. Dan ingat, jangan menjadi religius tentang nasihat ini juga.

Sangat kontroversial apakah ini 'lebih baik' dari pada yang biasa jika ... lupakan kasus yang ada. Tetapi jika Anda ingin mencoba sesuatu yang lain, ini adalah cara yang umum untuk melakukannya.

Masukkan kondisi Anda dalam objek dan masukkan objek-objek itu dalam daftar

foreach(var condition in Conditions.OrderBy(i=>i.OrderToRunIn))
{
    if(condition.EvaluatesToTrue())
    {
        addAlert(condition.Alert);
        break;
    }
}

Jika diperlukan beberapa tindakan dengan syarat Anda dapat melakukan rekursi gila

void RunConditionalAction(ConditionalActionSet conditions)
{
    foreach(var condition in conditions.OrderBy(i=>i.OrderToRunIn))
    {
        if(condition.EvaluatesToTrue())
        {
            RunConditionalAction(condition);
            break;
        }
    }
}

Jelas ya. Ini hanya berfungsi jika Anda memiliki pola pada logika Anda. Jika Anda mencoba untuk membuat tindakan kondisional rekursif super generik maka setup untuk objek akan serumit seperti pernyataan if asli. Anda akan menciptakan bahasa / kerangka kerja baru Anda sendiri.

Tetapi teladan Anda memang memiliki pola

Kasus penggunaan umum untuk pola ini adalah validasi. Dari pada :

bool IsValid()
{
    if(condition1 == false)
    {
        throw new ValidationException("condition1 is wrong!");
    }
    elseif(condition2 == false)
    {
    ....

}

Menjadi

[MustHaveCondition1]
[MustHaveCondition2]
public myObject()
{
    [MustMatchRegExCondition("xyz")]
    public string myProperty {get;set;}
    public bool IsValid()
    {
        conditions = getConditionsFromReflection()
        //loop through conditions
    }
}

Pertimbangkan untuk menggunakan return;setelah satu kondisi berhasil, itu akan menyelamatkan Anda semua else. Anda bahkan mungkin dapat return addAlert(1)secara langsung jika metode itu memiliki nilai balik.

Saya telah melihat konstruksi seperti ini terkadang dianggap lebih bersih:

switch(true) {
    case cond1(): 
        statement1; break;
    case cond2():
        statement2; break;
    case cond3():
        statement3; break;
    // .. etc
}

Ternary dengan spasi yang tepat juga bisa menjadi alternatif yang rapi:

cond1() ? statement1 :
cond2() ? statement2 :
cond3() ? statement3 : (null);

Saya kira Anda juga bisa mencoba membuat array dengan pasangan yang berisi kondisi dan fungsi dan beralih di atasnya sampai kondisi pertama terpenuhi - yang seperti yang saya lihat akan sama dengan jawaban pertama Ewan.

Sebagai varian dari jawaban @ Ewan, Anda dapat membuat rantai (alih-alih "daftar datar") dari kondisi seperti ini:

abstract class Condition {
  private static final  Condition LAST = new Condition(){
     public void alertOrPropagate(DisplayInterface display){
        // do nothing;
     }
  }
  private Condition next = Last;

  public Condition setNext(Condition next){
    this.next = next;
    return this; // fluent API
  }

  public void alertOrPropagate(DisplayInterface display){
     if(isConditionMeet()){
         display.alert(getMessage());
     } else {
       next.alertOrPropagate(display);
     }
  }
  protected abstract boolean isConditionMeet();
  protected abstract String getMessage();  
}

Dengan cara ini Anda dapat menerapkan kondisi Anda dalam urutan yang ditentukan dan infrastruktur (kelas abstrak yang ditunjukkan) melewatkan cek yang tersisa setelah yang pertama telah dipenuhi.

Di sinilah lebih unggul daripada pendekatan "daftar datar" di mana Anda harus menerapkan "lewati" dalam loop yang menerapkan kondisi.

Anda cukup mengatur rantai kondisi:

Condition c1 = new Condition1().setNext(
  new Condition2().setNext(
   new Condition3()
 )
);

Dan mulai evaluasi dengan panggilan sederhana:

c1.alertOrPropagate(display);

Pertama-tama, kode asli bukan IMO yang mengerikan. Ini cukup dimengerti dan tidak ada yang buruk secara inheren di dalamnya.

Kemudian jika Anda tidak suka, bangunlah ide @ Ewan untuk menggunakan daftar tetapi menghapus foreach breakpola yang agak tidak wajar :

public class conditions
{
    private List<Condition> cList;
    private int position;

    public Condition Head
    {
        get { return cList[position];}
    }

    public bool Next()
    {
        return (position++ < cList.Count);
    }
}


while not conditions.head.check() {
  conditions.next()
}
conditions.head.alert()

Sekarang sesuaikan ini dalam bahasa pilihan Anda, jadikan masing-masing elemen daftar objek, tupel, apa pun, dan Anda baik-baik saja.

EDIT: sepertinya tidak sejelas yang saya pikir, jadi izinkan saya menjelaskan lebih lanjut. conditionsadalah daftar yang diurutkan atau semacamnya; headadalah elemen saat ini sedang diselidiki - pada awalnya itu adalah elemen pertama dari daftar, dan setiap kali next()disebut itu menjadi yang berikut; check()dan alert()adalah checkConditionX()dan addAlert(X)dari OP.

Pertanyaannya kurang spesifik. Jika kondisinya:

• dapat berubah atau
• diulangi di bagian lain dari aplikasi atau sistem atau
• dimodifikasi dalam kasus-kasus tertentu (seperti bangunan yang berbeda, pengujian, penyebaran)

atau jika isinya addAlertlebih rumit, maka solusi yang mungkin lebih baik di katakanlah c # adalah:

//in some central spot
IEnumerable<Tuple<Func<bool>, int>> Conditions = new ... {
  Tuple.Create(CheckCondition1, 1),
  Tuple.Create(CheckCondition2, 2),
  ...
}

//at the original place
var matchingCondition = Conditions.Where(c=>c.Item1()).FirstOrDefault();
if(matchingCondition != null) 
  addAlert(matchingCondition.Item2)

Tuples tidak begitu cantik di c # <8, tetapi dipilih untuk kenyamanan.

Pro dengan metode ini, bahkan jika tidak ada opsi di atas yang berlaku, adalah bahwa struktur diketik secara statis. Anda tidak dapat secara tidak sengaja mengacaukan, katakanlah, melewatkan sebuah else.

Cara terbaik untuk mengurangi kompleksitas Siklomatik dalam kasus di mana Anda memiliki banyak if->then statementsadalah dengan menggunakan kamus atau daftar (tergantung bahasa) untuk menyimpan nilai kunci (jika nilai pernyataan atau beberapa nilai) dan kemudian hasil nilai / fungsi.

Misalnya, alih-alih (C #):

if (i > 10) { return "Two"; }
else if (i > 8) { return "Four" }
else if (i > 4) { return "Eight" }
return "Ten";  //etc etc say anything after 3 or 4 values

Saya bisa saja

var results = new Dictionary<int, string>
{
  { 10, "Two" },
  { 8, "Four"},
  { 4, "Eight"},
  { 0, "Ten"},
}

foreach(var key in results.Keys)
{
  if (i > results[key]) return results.Values[key];
}

Jika Anda menggunakan bahasa yang lebih modern Anda dapat menyimpan lebih banyak logika kemudian cukup nilai juga (c #). Ini benar-benar hanya fungsi sebaris, tetapi Anda juga bisa hanya menunjuk ke fungsi lain juga jika logikanya adalah untuk menempatkan secara in-line.

var results = new Dictionary<Func<int, bool>, Func<int, string>>
{
  { (i) => return i > 10; ,
    (i) => return i.ToString() },
  // etc
};

foreach(var key in results.Keys)
{ 
  if (key(i)) return results.Values[key](i);
}

Saya mencoba mengikuti saran kode bersih Paman Bob dan secara khusus membuat metode singkat.

Saya menemukan diri saya tidak dapat mempersingkat logika ini:

if (checkCondition()) {addAlert(1);}
else if (checkCondition2()) {addAlert(2);}
else if (checkCondition3()) {addAlert(3);}
else if (checkCondition4()) {addAlert(4);}

Kode Anda sudah terlalu pendek, tetapi logikanya sendiri tidak boleh diubah. Pada pandangan pertama sepertinya Anda mengulangi diri Anda dengan empat panggilan checkCondition(), dan itu hanya jelas bahwa masing-masing berbeda setelah membaca ulang kode dengan hati-hati. Anda harus menambahkan nama pemformatan dan fungsi yang tepat, misalnya:

if (is_an_apple()) {
  addAlert(1);
}
else if (is_a_banana()) {
  addAlert(2);
}
else if (is_a_cat()) {
  addAlert(3);
}
else if (is_a_dog()) {
  addAlert(4);
}

Kode Anda harus dapat dibaca di atas segalanya. Setelah membaca beberapa buku Paman Bob, saya percaya itu adalah pesan yang secara konsisten ia coba sampaikan.

Misalkan semua fungsi diimplementasikan dalam komponen yang sama, Anda bisa membuat fungsi mempertahankan beberapa negara untuk menyingkirkan beberapa cabang dalam aliran.

EG: checkCondition1()akan menjadi evaluateCondition1(), di mana ia akan memeriksa apakah kondisi sebelumnya terpenuhi; jika demikian, maka cache beberapa nilai untuk diambil oleh getConditionNumber().

checkCondition2()akan menjadi evaluateCondition2(), di mana ia akan memeriksa apakah kondisi sebelumnya terpenuhi. Jika kondisi sebelumnya tidak terpenuhi, maka memeriksa kondisi skenario 2, menyimpan nilai yang akan diambil getConditionNumber(). Dan seterusnya.

clearConditions();
evaluateCondition1();
evaluateCondition2();
evaluateCondition3();
evaluateCondition4();
if (anyCondition()) { addAlert(getConditionNumber()); }

SUNTING:

Berikut adalah cara memeriksa kondisi yang mahal perlu diterapkan agar pendekatan ini berhasil.

bool evaluateCondition34() {
    if (!anyCondition() && A && B && C) {
        conditionNumber = 5693;
        return true;
    }
    return false;
}

...

bool evaluateCondition76() {
    if (!anyCondition() && !B && C && D) {
        conditionNumber = 7658;
        return true;
    }
    return false;
}

Oleh karena itu, jika Anda memiliki terlalu banyak pemeriksaan mahal untuk dilakukan, dan hal-hal dalam kode ini tetap pribadi, pendekatan ini membantu mempertahankannya, memungkinkan untuk mengubah urutan pemeriksaan jika perlu.

clearConditions();
evaluateCondition10();
evaluateCondition9();
evaluateCondition8();
evaluateCondition7();
...
evaluateCondition34();
...
evaluateCondition76();

if (anyCondition()) { addAlert(getConditionNumber()); }

Jawaban ini hanya memberikan beberapa saran alternatif dari jawaban yang lain, dan mungkin tidak akan lebih baik dari kode asli jika kita anggap hanya 4 baris kode. Meskipun, ini bukan pendekatan yang mengerikan (dan tidak ada yang membuat pemeliharaan lebih sulit seperti yang dikatakan orang lain) mengingat skenario yang saya sebutkan (terlalu banyak pemeriksaan, hanya fungsi utama yang ditampilkan sebagai publik, semua fungsi adalah rincian implementasi dari kelas yang sama).

Adakah lebih dari dua klausul "lain" memaksa pembaca kode untuk menelusuri seluruh rantai untuk menemukan yang menarik. Gunakan metode seperti: void AlertUponCondition (Kondisi kondisi) {switch (kondisi) {case Condition.Con1: ... break; kondisi case.Con2: ... istirahat; dll ...} Di mana "Kondisi" adalah enum yang tepat. Jika perlu, kembalikan bool atau nilai. Sebut saja seperti ini: AlertOnCondition (GetCondition ());

Ini benar-benar tidak bisa lebih sederhana, DAN itu lebih cepat daripada rantai if-else setelah Anda melebihi beberapa kasus.

Saya tidak dapat berbicara untuk situasi khusus Anda karena kode ini tidak spesifik, tetapi ...

kode seperti itu sering menjadi bau bagi model OO yang kurang. Anda benar-benar memiliki empat jenis hal, masing-masing terkait dengan jenis alerternya sendiri, tetapi alih-alih mengenali entitas ini dan membuat instance kelas untuk masing-masing, Anda memperlakukannya sebagai satu hal dan mencoba menebusnya nanti, pada saat Anda benar-benar perlu tahu apa yang Anda hadapi untuk melanjutkan.

Polimorfisme mungkin lebih cocok untuk Anda.

Curiga terhadap kode dengan metode panjang yang berisi konstruksi panjang atau rumit jika-maka. Anda sering menginginkan pohon kelas di sana dengan beberapa metode virtual.