Untuk-jika antipattern

9

Saya membaca di posting blog ini tentang anti-pola anti-jika, dan saya tidak yakin saya mengerti mengapa ini anti-pola.

foreach (string filename in Directory.GetFiles("."))
{
    if (filename.Equals("desktop.ini", StringComparison.OrdinalIgnoreCase))
    {
        return new StreamReader(filename);
    }
}

Pertanyaan 1:

Apakah karena return new StreamReader(filename);di dalam for loop? atau fakta bahwa Anda tidak memerlukan forloop dalam kasus ini?

Sebagai penulis blog menunjukkan versi kurang gila dari ini adalah:

if (File.Exists("desktop.ini"))
{
    return new StreamReader("desktop.ini");
}

keduanya menderita kondisi balapan karena jika file dihapus sebelum pembuatan StreamReader, Anda akan mendapatkan File­Not­Found­Exception.

Pertanyaan 2:

Untuk memperbaiki contoh kedua, apakah Anda akan menulis ulang tanpa pernyataan if, dan bukannya mengelilingi StreamReaderdengan blok try-catch, dan jika itu melempar File­Not­Found­ExceptionAnda menangani di catchblok sesuai?

c# anti-patterns
sumber
1
lihat Diskusikan $ {blog} ini
nyamuk
1
@amon - Terima kasih, tapi saya tidak membuat poin tambahan, saya hanya mencoba memahami apa yang dikatakan artikel dan bagaimana saya akan memperbaikinya.
3
Saya tidak akan banyak memikirkan ini. Poster blog membuat beberapa kode idiot yang tidak seorang pun menulis, memberinya nama dan menyebutnya anti-pola. Ini satu lagi: "Saya menyebutnya pola penugasan tanpa tujuan: x = x; Saya melihat ini dilakukan setiap saat. Bodoh sekali. Saya pikir saya akan menulis blog tentang itu."
Martin Maat
4
To fix the second example, would you re-write it without the if statement, and instead surround the StreamReader with a try-catch block, and if it throws a File­Not­Found­Exception you handle it in the catch block accordingly?- Ya, itulah yang akan saya lakukan. Memecahkan kondisi balapan lebih penting daripada beberapa gagasan "pengecualian sebagai aliran kontrol," dan menyelesaikannya dengan elegan dan bersih.
Robert Harvey
1
Apa yang dilakukan jika tidak ada file yang memenuhi kriteria yang diberikan? Apakah ini kembali null? Anda biasanya dapat menggunakan LINQ untuk membersihkan kode yang terlihat seperti kode contoh Anda: return Directory.GetFiles(".").FirstOrDefault(fileName => fileName.Equals("desktop.ini", StringComparison.OrdinalIgnoreCase))?.Select(fileName => new StreamReader(filename)); Perhatikan ?.operator di antara dua panggilan LINQ. Juga orang mungkin berpendapat bahwa pembuatan objek seperti ini bukan penggunaan LINQ yang paling tepat, tapi saya pikir tidak apa-apa di sini. Ini bukan jawaban untuk pertanyaan Anda, tetapi apakah menyimpang pada satu bagian dari itu.
Panzercrisis

Jawaban:

7

Ini adalah antipattern karena mengambil bentuk:

loop over a set of values
   if current value meets a condition
       do something with value
   end
end

dan bisa diganti dengan 

do something with value

Contoh klasik dari ini adalah kode seperti:

for (var i=0; i < 5; i++)
{
    switch (i)
        case 1:
            doSomethingWith(1);
            break;
        case 2:
            doSomethingWith(2);
            break;
        case 3:
            doSomethingWith(4);
            break;
        case 4:
            doSomethingWith(4);
            break;
    }
}

Ketika yang berikut ini berfungsi dengan baik:

doSomethingWith(1);
doSomethingWith(2);
doSomethingWith(3);
doSomethingWith(4);

Jika Anda menemukan diri Anda melakukan perulangan dan ifatau switch, kemudian berhenti dan pikirkan apa yang Anda lakukan. Apakah Anda overcomplicating hal dan dapat seluruh loop dan tes hanya diganti dengan garis sederhana "lakukan saja". Namun kadang-kadang, Anda perlu melakukan loop itu (misalnya, lebih dari satu item cocok dengan satu syarat), dalam hal ini polanya baik-baik saja.

Itulah mengapa ini merupakan anti-pola: dibutuhkan pola "loop and test" dan melecehkannya.

Mengenai pertanyaan kedua Anda: ya. Pola "coba lakukan" lebih kuat daripada pola "tes lalu lakukan" dalam situasi apa pun di mana kode Anda bukan satu-satunya utas pada seluruh perangkat yang dapat mengubah status item yang diuji.

Masalah dengan kode ini:

if (File.Exists("desktop.ini"))
{
    return new StreamReader("desktop.ini");
}

adalah bahwa di antara waktu File.Existsdan StreamReadermencoba untuk membuka file itu, utas atau proses lain dapat menghapus file. Jadi, Anda akan mendapatkan pengecualian. Karena itu pengecualian itu harus dijaga melalui sesuatu seperti:

try
{
    return new StreamReader("desktop.ini");
}
catch (File­Not­Found­Exception)
{
    return null; // or whatever
}

@Flater memunculkan poin yang bagus? Apakah ini sendiri merupakan antipattern? Apakah kita menggunakan pengecualian sebagai aliran kontrol?

Jika kode membaca sesuatu seperti:

try
{
    if (!File.Exists("desktop.ini")
    {
        throw new IniFileMissingException();
        return new StreamReader("desktop.ini");
    }
}
catch (IniFileMissingException)
{
    return null;
}

maka saya memang akan menggunakan pengecualian sebagai kebanggaan yang dimuliakan dan itu memang akan menjadi anti-pola. Tetapi dalam kasus ini kami hanya mengatasi perilaku yang tidak diinginkan dalam menciptakan aliran baru, jadi ini bukan contoh dari pola anti itu. Tetapi ini adalah contoh dari mengatasi anti-pola itu.

Tentu saja, yang benar-benar kita inginkan adalah cara yang lebih elegan untuk menciptakan aliran. Sesuatu seperti:

return TryCreateStream("desktop.ini", out var stream) ? stream : null;

Dan saya sarankan membungkus try catchkode itu dalam metode utilitas seperti ini jika Anda sering menggunakan kode ini.

David Arno
sumber
1
@Flater, saya cenderung setuju bahwa itu tidak ideal (meskipun saya membantah itu adalah contoh dari anti-pola yang menjawab pembicaraan). Kode harus menunjukkan maksudnya, bukan mekanika. Jadi saya mendukung sesuatu seperti milik Scala Try, misalnya return Try(() => new StreamReader("desktop.ini")).OrElse(null);, tetapi C # tidak mendukung konstruk itu (tanpa menggunakan perpustakaan pihak ke-3), jadi kami harus bekerja dengan versi kikuk.
David Arno
1
@Flater, saya sepenuhnya setuju bahwa pengecualian harus luar biasa. Tetapi mereka jarang. Misalnya, file yang tidak ada hampir tidak luar biasa, namun File.Openakan membuangnya jika tidak dapat menemukan file. Karena kita sudah memiliki pengecualian yang tidak biasa yang dilemparkan, menjebaknya sebagai bagian dari aliran kontrol adalah suatu keharusan (kecuali seseorang ingin membiarkan aplikasi itu crash).
David Arno
1
@Flater: contoh kode kedua adalah cara yang tepat untuk membuka file. Ada lagi yang memperkenalkan kondisi balapan. Bahkan jika Anda memeriksa keberadaan file, antara cek itu dan ketika Anda benar-benar membukanya, sesuatu dapat membuat file itu tidak tersedia untuk Anda, dan panggilan Open () akan meledak. Jadi, Anda harus siap untuk pengecualian. Jadi Anda mungkin juga tidak perlu repot memeriksa dan hanya mencoba dan membukanya. Pengecualian adalah alat untuk membantu kami menyelesaikan pekerjaan dan penggunaannya tidak harus dipandang sebagai dogma agama.
whatsisname
1
@Flater: cara apa pun untuk menghindari mencoba / menangkap sekitar operasi file memperkenalkan kondisi balapan. Sistem file yang ingin Anda tulis bisa menjadi tidak tersedia secara instan sebelum Anda memanggil File.Create, menjadikan semua pemeriksaan tidak valid.
whatsisname
1
@Flater " Anda secara efektif berargumen bahwa setiap pemanggilan metode memerlukan jenis kembali ... yang secara efektif mencoba untuk menemukan kembali pengecualian dengan cara yang berbeda ". Benar. Meskipun reinvention itu bukan milikku. Ini telah digunakan dalam bahasa fungsional selama bertahun-tahun. Mereka umumnya menghindari keseluruhan "pengecualian sebagai masalah aliran kontrol" (yang secara membingungkan Anda klaim adalah anti-pola dalam satu nafas dan kemudian berdebat dalam mendukung berikutnya) dengan menggunakan jenis serikat, misalnya dalam kasus ini kami akan menggunakan Maybe<Stream>jenis yang mengembalikan nothingjika file tidak ada dan aliran jika itu.
David Arno
1

Pertanyaan 1: (apakah ini untuk loop sebuah antipattern)

Ya, karena Anda tidak perlu melakukan pencarian sendiri untuk item yang disimpan dalam subsistem yang dapat ditanyakan.

Dalam abstrak, sistem file, seperti database, mampu merespons permintaan. Saat berinteraksi dengan subsistem yang dapat ditelusuri, kami memiliki pilihan mendasar, apakah kami ingin subsistem tersebut untuk menyebutkan isinya kepada kami agar kami dapat melakukan pencocokan di luar subsistem, atau menggunakan kemampuan kueri asli subsistem.

Mari kita berpura-pura sejenak bahwa Anda sedang mencari catatan dalam database daripada file dalam direktori di sistem file. Apakah Anda lebih suka melihat

SELECT * FROM SomeTable;

dan kemudian dalam loop (misalnya dalam C #) di atas kursor yang dikembalikan mencari ID = 100, atau membiarkan subsistem yang dapat diminta melakukan apa yang dapat dilakukan untuk menemukan apa yang Anda cari?

SELECT * FROM SomeTable WHERE ID = 100;

Saya harus berpikir bahwa sebagian besar dari kita akan memilih untuk membiarkan subsistem melakukan permintaan yang tepat. Alternatif ini melibatkan beberapa perjalanan bolak-balik yang berpotensi dengan subsistem, pengujian kesetaraan yang tidak efisien, dan menghentikan penggunaan indeks atau akselerator pencarian lainnya, yang disediakan baik oleh basis data dan sistem file.

Pertanyaan 2: Untuk memperbaiki contoh kedua, apakah Anda akan menulis ulang tanpa pernyataan if, dan bukannya mengelilingi StreamReader dengan blok try-catch, dan jika ia melempar FileNotFoundException Anda menanganinya di blok catch sesuai?

Ya, karena itulah cara API tertentu bekerja - sebenarnya bukan pilihan kami karena ini adalah fungsi perpustakaan. Pemeriksaan if, sebelum panggilan, tidak memberikan nilai tambahan: kita harus tetap menggunakan try / catch, karena (1) kesalahan lain di luar FileNotFound dapat terjadi, dan (2) kondisi balapan.

Erik Eidt
sumber
0

Pertanyaan 1:

Apakah karena mengembalikan StreamReader (nama file) baru; di dalam for loop? atau fakta bahwa Anda tidak memerlukan loop for dalam kasus ini?

Streamreader tidak ada hubungannya dengan itu. Anti-pola muncul karena konflik niat yang jelas antara foreachdan if:

Apa tujuan dari foreach?

Saya berasumsi jawaban Anda akan seperti: "Saya ingin berulang kali mengeksekusi kode tertentu"

Berapa banyak file yang Anda harapkan diproses?

Karena Anda hanya dapat memiliki satu nama file tertentu (termasuk ekstensi) di folder tertentu, ini membuktikan bahwa kode Anda dimaksudkan untuk menemukan satu file yang berlaku.

Ini juga dikonfirmasi oleh fakta bahwa Anda segera mengembalikan nilai. Anda sebenarnya tidak peduli dengan pertandingan kedua, bahkan jika itu ada.

Ada situasi di mana ini bukan anti-pola.

  • Jika Anda juga mencari di subdirektori ( Directory.GetFiles(".", SearchOption.AllDirectories)), maka dimungkinkan untuk menemukan lebih dari satu file dengan nama file yang sama (termasuk ekstensi)
  • Jika Anda mencari kecocokan sebagian nama file (mis. Setiap file yang namanya dimulai dengan "Test_", atau setiap "*.zip"file.

Perhatikan bahwa kedua kasus ini mengharuskan Anda untuk benar-benar memproses beberapa pertandingan dan karenanya tidak segera mengembalikan nilai.

Pertanyaan 2:

Untuk memperbaiki contoh kedua, apakah Anda akan menulis ulang tanpa pernyataan if, dan alih-alih mengelilingi StreamReader dengan blok coba-tangkap, dan jika melempar FileNotFoundException Anda menanganinya di blok tangkap sesuai?

Pengecualian itu mahal. Mereka tidak boleh digunakan sebagai pengganti logika aliran yang tepat. Pengecualian, seperti namanya menunjukkan keadaan luar biasa .

Karena alasan itu, Anda tidak boleh menghapus if.

Sesuai jawaban ini di SoftwareEngineering.SE :

Secara umum, penggunaan pengecualian untuk aliran kontrol adalah anti-pola, dengan pengecualian batuk khusus situasi dan bahasa.

Sebagai ringkasan singkat mengapa, pada umumnya, ini merupakan anti-pola:

  • Pengecualian pada dasarnya adalah pernyataan GOTO yang canggih
  • Pemrograman dengan pengecualian, oleh karena itu, menyebabkan lebih sulit untuk membaca, dan memahami kode
  • Sebagian besar bahasa memiliki struktur kontrol yang dirancang untuk menyelesaikan masalah Anda tanpa menggunakan pengecualian
  • Argumen untuk efisiensi cenderung diperdebatkan untuk kompiler modern, yang cenderung untuk mengoptimalkan dengan asumsi bahwa pengecualian tidak digunakan untuk aliran kontrol.

Baca diskusi di wiki Ward untuk informasi lebih mendalam.

Apakah Anda perlu membungkus ini dalam percobaan / tangkapan, sangat tergantung pada situasi Anda:

  • Seberapa besar kemungkinan Anda akan menghadapi kondisi balapan?
  • Apakah Anda benar-benar dapat menangani situasi ini, atau Anda ingin masalah ini meluap kepada pengguna karena Anda tidak tahu bagaimana menanganinya.

Tidak ada yang pernah menjadi masalah "selalu gunakan itu". Untuk membuktikan maksud saya:

Studi terpisah telah membuktikan bahwa Anda tidak akan terluka saat mengenakan helm pengaman, kacamata keselamatan, dan rompi anti peluru.

Jadi mengapa kita tidak memakai peralatan keselamatan ini setiap saat?

Jawaban sederhana adalah karena ada kekurangan untuk memakainya:

  • Membutuhkan uang
  • Itu membuat gerakan Anda lebih rumit
  • Bisa jadi cukup hangat untuk memakainya.

Sekarang kita berada di suatu tempat: ada pro dan kontra . Dengan kata lain, masuk akal untuk memakai peralatan ini dalam kasus di mana pro lebih besar daripada kontra.

  • Pekerja konstruksi jauh lebih mungkin menderita cedera selama pekerjaan mereka. Mereka mendapat manfaat dari helm pengaman.
  • Pekerja kantor, di sisi lain, memiliki peluang yang jauh lebih rendah untuk mengalami cedera. Helm pengaman tidak sepadan.
  • Seorang anggota tim SWAT lebih mungkin untuk tertembak, dibandingkan dengan pekerja kantor.

Haruskah Anda membungkus panggilan dengan mencoba / menangkap? Itu sangat tergantung pada apakah manfaat melakukan itu lebih besar daripada biaya pelaksanaannya.

Perhatikan bahwa orang lain mungkin berpendapat bahwa hanya perlu beberapa penekanan tombol untuk membungkusnya, jadi itu jelas harus dilakukan. Tapi itu bukan keseluruhan argumen:

  • Anda perlu memutuskan apa yang harus dilakukan setelah Anda menangkap pengecualian.
  • Jika ada banyak panggilan berbeda ke file yang berbeda di seluruh basis kode, memutuskan untuk membungkus satu dalam percobaan / tangkapan umumnya akan berarti bahwa Anda harus membungkus semua kasus ini. Ini dapat memiliki efek dramatis pada jumlah upaya yang diperlukan untuk mengimplementasikannya.
  • Ini sangat mungkin bahwa Anda sengaja ingin tidak menangani pengecualian.
    • Perhatikan bahwa aplikasi Anda harus menangani pengecualian di beberapa titik, tetapi itu tidak harus segera setelah pengecualian itu muncul.

Jadi pilihan ada di tangan Anda. Apakah ada manfaatnya? Apakah Anda pikir itu meningkatkan aplikasi, lebih dari upaya biaya untuk mengimplementasikannya?

Pembaruan - Dari komentar yang saya tulis ke jawaban lain, karena saya pikir itu juga merupakan pertimbangan yang relevan untuk Anda:

Ini sangat bergantung pada konteks sekitarnya.

  • Jika pembukaan streamreader didahului oleh if(!File.Exists) File.Create(), maka tidak adanya file saat membuka streamreader memang luar biasa .
  • Jika nama file dipilih dari daftar file yang ada, ketiadaan tiba-tiba sekali lagi luar biasa .
  • Jika Anda bekerja dengan string yang belum benar-benar diuji terhadap direktori; maka tidak adanya file adalah hasil yang sangat logis, dan karena itu tidak luar biasa .
Flater
sumber