Maintainability of Boolean logic - Apakah bersarang jika pernyataan dibutuhkan?

Manakah dari ini yang lebih baik untuk pemeliharaan?

if (byteArrayVariable != null)
    if (byteArrayVariable .Length != 0)
         //Do something with byteArrayVariable 

ATAU

if ((byteArrayVariable != null) && (byteArrayVariable.Length != 0))
  //Do something with byteArrayVariable 

Saya lebih suka membaca dan menulis yang kedua, tetapi saya ingat membaca dalam kode lengkap bahwa melakukan hal-hal seperti itu buruk untuk pemeliharaan.

Ini karena Anda mengandalkan bahasa untuk tidak mengevaluasi bagian kedua ifjika bagian pertama salah dan tidak semua bahasa melakukannya. (Bagian kedua akan melempar pengecualian jika dievaluasi dengan nol byteArrayVariable.)

Saya tidak tahu apakah itu benar-benar sesuatu yang perlu dikhawatirkan atau tidak, dan saya ingin umpan balik umum atas pertanyaan itu.

Terima kasih.

Saya pikir bentuk kedua baik-baik saja, dan juga lebih jelas mewakili apa yang Anda coba lakukan. Itu kata kamu...

Saya ingat membaca dalam kode lengkap bahwa melakukan hal-hal seperti itu buruk untuk pemeliharaan. Ini karena Anda mengandalkan bahasa untuk tidak mengevaluasi bagian kedua jika jika bagian pertama salah dan tidak semua bahasa melakukannya.

Tidak masalah jika semua bahasa melakukan itu. Anda menulis berada di satu bahasa tertentu, sehingga hanya penting jika itu bahasa melakukan itu. Jika tidak, Anda pada dasarnya mengatakan bahwa Anda tidak boleh menggunakan fitur-fitur bahasa tertentu karena bahasa lain mungkin tidak mendukung fitur-fitur itu, dan itu konyol.

Saya terkejut tidak ada yang menyebutkannya (jadi saya harap saya tidak salah membaca pertanyaan) - tetapi keduanya payah!

Alternatif yang lebih baik adalah dengan menggunakan keluar awal (menempatkan pernyataan kembali di awal kode jika tidak ada kode lain yang harus dijalankan). Ini membuat asumsi bahwa kode Anda relatif baik dire-refored, dan bahwa setiap metode memiliki tujuan yang ringkas.

Pada titik itu Anda akan melakukan sesuatu seperti:

if ((byteArrayVariable == null) || (byteArrayVariable.Length != 0)) {
  return; // nothing to be done, leaving
}

// Conditions met, do something

Ini mungkin terlihat seperti validasi - dan memang seperti itu. Ini memvalidasi bahwa kondisi telah dipenuhi untuk metode untuk melakukan hal itu - kedua opsi yang Anda tawarkan menyembunyikan logika sebenarnya dalam pemeriksaan pra-kondisi.

Jika kode Anda banyak spaghetti (metode loooong, banyak ifs bersarang dengan logika tersebar di antara mereka, dll) maka Anda harus fokus pada masalah yang lebih besar untuk mendapatkan kode Anda sebelum masalah gaya yang lebih kecil. Tergantung pada lingkungan Anda, dan tingkat keparahan kekacauan, ada beberapa alat yang hebat untuk membantu (misalnya Visual Studio memiliki fungsi refactoring "Metode Ekstrak").

Seperti yang disebutkan Jim, masih ada ruang untuk debat tentang bagaimana menyusun pintu keluar awal. Alternatif untuk apa yang di atas adalah:

if (byteArrayVariable == null) 
  return; // nothing to be done, leaving

if (byteArrayVariable.Length != 0)
  return;

// Conditions met, do something

Jangan buang waktu mencoba menjebak untuk setiap kondisi ini . Jika Anda dapat mendiskualifikasi data atau ketentuan tersebut, lakukan sesegera mungkin.

if (byteArrayVariable == null) Exit;
if (byteArrayVariable.Length == 0) Exit;
// do something

Ini memungkinkan Anda mengadaptasi kode Anda lebih mudah untuk kondisi baru

if (byteArrayVariable == null) Exit;
if (byteArrayVariable.Length == 0) Exit;
if (NewCondition == false) Exit;
if (NewerCondition == true) Exit;
// do something

Contoh Anda adalah contoh sempurna mengapa nested ifsadalah pendekatan yang buruk untuk sebagian besar bahasa yang mendukung perbandingan boolean. Bersarang ifsmenciptakan situasi di mana niat Anda sama sekali tidak jelas. Apakah diharuskan yang kedua ifsegera mengikuti yang pertama? Atau itu hanya karena Anda belum melakukan operasi lain di sana?

if ((byteArrayVariable != null) && (byteArrayVariable.Length != 0))
  //Do something with byteArrayVariable 

Dalam hal ini, ini cukup banyak harus bersama. Mereka bertindak sebagai penjaga untuk memastikan Anda tidak melakukan operasi yang akan menghasilkan pengecualian. Dengan menggunakan nested ifs, Anda tinggal menafsirkan orang yang mengikuti Anda, apakah keduanya mewakili penjaga dua bagian atau logika percabangan lainnya. Dengan menggabungkan mereka menjadi satu ifsaya menyatakan itu. Jauh lebih sulit untuk menyelinap operasi di dalam sana di mana seharusnya tidak.

Saya akan selalu mendukung komunikasi yang jelas tentang niat saya atas pernyataan bodoh seseorang bahwa itu "tidak berfungsi dalam semua bahasa". Coba tebak ... throwtidak berfungsi dalam semua bahasa, apakah itu berarti saya tidak boleh menggunakannya saat mengkode dalam C # atau Java dan sebaliknya harus bergantung pada nilai pengembalian untuk memberi sinyal ketika ada masalah?

Semua yang dikatakan, itu jauh lebih mudah dibaca untuk membalikkan dan hanya keluar dari metode jika salah satu tidak puas seperti kata STW dalam jawaban mereka.

Dalam hal ini dua klausa Anda terkait langsung, jadi bentuk ke-2 lebih disukai.

Jika mereka tidak terkait (misalnya serangkaian klausa penjaga pada kondisi yang berbeda) maka saya lebih suka bentuk 1 (atau saya akan refactor metode dengan nama yang mewakili apa kondisi senyawa sebenarnya mewakili).

Jika Anda bekerja di Delphi, cara pertama adalah, secara umum, lebih aman. (Untuk contoh yang diberikan, tidak ada banyak manfaat dari menggunakan ifs bersarang.)

Delphi memungkinkan Anda menentukan apakah ekspresi boolean mengevaluasi atau "korsleting", melalui sakelar kompiler. Melakukan cara # 1 (jika bersarang) memungkinkan Anda untuk memastikan bahwa klausa kedua dijalankan jika dan hanya jika (dan setelahnya ) klausa pertama dievaluasi menjadi true.

Gaya kedua dapat menjadi bumerang di VBA karena jika tes pertama adalah jika objek dihasilkan, masih akan melakukan tes kedua dan keluar kesalahan. Saya baru saja terbiasa dengan VBA ketika berurusan dengan verifikasi objek untuk selalu menggunakan metode pertama.

Bentuk kedua lebih mudah dibaca, terutama jika Anda menggunakan {} blok di sekitar setiap klausa, karena bentuk pertama akan mengarah ke blok {} bersarang dan berbagai tingkat lekukan, yang bisa jadi sulit dibaca (Anda harus menghitung ruang indentasi untuk mencari tahu di mana masing-masing blok berakhir).

Saya menemukan keduanya dapat dibaca, dan saya tidak menerima argumen bahwa Anda harus khawatir tentang pemeliharaan kode jika Anda berganti bahasa.

Dalam beberapa bahasa opsi kedua berkinerja lebih baik sehingga itu akan menjadi pilihan yang jelas.

Aku bersamamu; Saya melakukannya dengan cara kedua. Bagi saya, secara logis lebih jelas. Kekhawatiran bahwa beberapa bahasa akan menjalankan bagian kedua bahkan jika yang pertama dinilai salah sepertinya agak konyol bagi saya, karena dalam hidup saya belum pernah menulis sebuah program yang berjalan dalam "beberapa bahasa"; kode saya sepertinya selalu ada dalam bahasa tertentu, yang bisa menangani konstruksi itu atau tidak. (Dan jika saya tidak yakin apakah bahasa tertentu dapat mengatasinya, itu adalah sesuatu yang benar-benar perlu saya pelajari, bukan.)

Dalam pikiran saya, bahaya dengan cara pertama melakukannya adalah membuat saya rentan untuk secara tidak sengaja memasukkan kode di luar ifblok bersarang ketika saya tidak bermaksud melakukannya. Yang, diakui, hampir tidak menjadi perhatian besar, tetapi tampaknya lebih masuk akal daripada khawatir apakah kode saya agnostik bahasa.

Saya lebih suka opsi kedua, karena lebih mudah dibaca.

Jika logika yang Anda terapkan lebih kompleks (memeriksa bahwa string bukan nol dan bukan kosong hanyalah contoh), maka Anda dapat membuat refactoring yang bermanfaat: ekstrak fungsi boolean. Saya dengan @mipadi pada komentar "Kode Selesai" ("tidak masalah jika semua bahasa melakukan itu ...") Jika pembaca kode Anda tidak tertarik melihat ke dalam fungsi yang diekstrak, maka urutannya di mana operan boolean dievaluasi menjadi pertanyaan yang diperdebatkan bagi mereka.

if ((byteArrayVariable != null)
 && (byteArrayVariable.Length != 0)) {
    //Do something with byteArrayVariable 

tapi itu pada dasarnya pilihan kedua.