Memeriksa hasil konstruktor di C #

8

Saya sedang mengerjakan basis kode dengan rekan kerja yang memiliki kebiasaan memeriksa hasil konstruktor untuk null dengan cara yang mirip dengan ini

Person p = new Person();
if (p != null)
{
    p.Name = "John Smith";
}

Pemahaman saya tentang lanskap .NET adalah bahwa konstruktor tidak akan pernah meninggalkan tugas tidak terpenuhi kecuali ada pengecualian yang dilemparkan. Jadi dalam kasus di atas, cek nol tidak berguna. Baik pdialokasikan, atau pengecualian akan dilemparkan menyebabkan penyetel properti dilewati.

Saya telah bertanya kepada rekan kerja saya tentang ini secara sepintas, dan saya mendapatkan jawaban pasif di sepanjang baris "berjaga-jaga". Saya sendiri tidak menyukai "pemrograman paronia" semacam ini. Saya pikir itu menyakitkan keterbacaan dan tidak perlu meningkatkan kompleksitas siklomatik. Karena itu, saya ingin mengajukan permintaan resmi agar praktik ini dihentikan. Apakah ini permintaan yang masuk akal? Apakah saya melewatkan sesuatu?

Jason Tyler
sumber
1
Sepertinya itu harus peringatan kompiler minimal, atau peringatan analisis kode.
Frank Hileman
@ FrankHileman - poin bagus ... kompiler C # memiliki peringatan (0161) tentang kode yang tidak dapat dijangkau, walaupun saya tidak 100% yakin ini dapat mendeteksi kasus khusus ini.
Jules
apakah ini benar-benar praktik standar untuk mendapatkan cek, atau hanya tersisa dari versi sebelumnya? yang mungkin memiliki p = Repository.Get (123) atau sesuatu
Ewan

Jawaban:

12

Saya setuju dengan @MartinMaat tentang memilih pertempuran Anda.

Ada banyak kasus "berjaga-jaga" karena tidak benar-benar memahami bahasa meskipun bahasa telah diperbaiki dalam aturannya untuk banyak hal ini - alih-alih mengurung ekspresi yang tidak memerlukannya karena tidak memahami aturan diutamakan dari bahasa. Tapi tetap saja, praktik seperti itu sebagian besar tidak berbahaya.

Ketika saya masih muda, saya merasa bahwa kita harus mempelajari perincian bahasa dan dengan demikian menghindari penulisan kode yang berlebihan. (Salah satu kencing binatang peliharaan saya adalah return (0);dengan parens yang tidak perlu.) Namun, saya sekarang memoderasi posisi itu, khususnya, karena kami menggunakan begitu banyak bahasa sekarang, melompat dari klien ke server, dll ... Jadi, saya sekarang memotong beberapa kendur untuk beberapa masalah seperti itu.

Anda poin tentang cyclomatic mulai pergi ke argumen yang beralasan logis. Mari kita lihat Kode Cakupan dan khususnya tingkat cakupan yang lebih tinggi :

  1. Setiap keputusan mengambil setiap kemungkinan hasil

Karena kami tidak dapat memaksa operasi baru untuk mengembalikan NULL, tidak ada cara untuk mencapai tingkat cakupan kode yang lebih tinggi untuk operasi bersyarat ini.   Tentu saja, ini mungkin atau mungkin tidak penting bagi organisasi Anda!

Namun, karena masalah cakupan kode ini, saya akan memprioritaskannya lebih tinggi daripada terlalu banyak tanda kurung.

Di sisi lain, kode yang dihasilkan mungkin tidak akan menderita sedikitpun karena kode generasi, JIT, dan pengoptimal semua mengerti bahwa newnilai ed tidak akan pernah menjadi nol. Jadi, biaya sebenarnya hanya datang dalam hal kemampuan membaca dan cakupan kode sumber.

Saya ingin bertanya seperti apa bentuk "bagian lain" dari pernyataan if itu?

Jika tidak ada bagian-lain, saya berpendapat bahwa hanya jatuh dari akhir rutin atau jatuh ke kode lain (yaitu tidak elseuntuk ini if) berpotensi berbahaya, karena sekarang ini "berjaga-jaga" secara logis menunjukkan bahwa penelepon dan / atau kode lebih lanjut di baris menangani NULL juga.

Jika berbunyi:

p = new Object ();
if ( p != null ) {
    p.field = value;
}
else {
    throw new NullReferenceException ();
}

maka ini benar-benar berlebihan, karena bahasa melakukan semua itu untuk kita.

Saya mungkin menyarankan membalikkan rasa bersyarat - mungkin rekan Anda akan lebih nyaman dengan ini:

p = new Object ();
if ( p == null ) {
    throw new NullReferenceException ();
}
else {
    p.field = value;
}

Sekarang Anda dapat berdebat untuk menghapus bungkus lain, karena itu jelas tidak perlu:

p = new Object ();
if ( p == null ) {
    throw new NullReferenceException ();
}
p.field = value;

Dengan ini, "berjaga-jaga" sekarang apa yang bersyarat, sedangkan kode berikutnya tidak. Pendekatan ini semakin memperkuat bahwa ketika alokasi gagal, respons yang sesuai adalah melempar, daripada terus menjalankan kode dalam metode ini dan / atau dalam rantai panggilan ini (tanpa penanganan kegagalan alokasi lainnya yang tepat).

Jadi, secara ringkas ada dua argumen yang masuk akal untuk dibuat di sini terhadap praktik ini:

  1. Level cakupan kode yang lebih tinggi tidak dapat dicapai karena kami tidak dapat memaksakan kehabisan memori (atau kegagalan konstruktor) untuk mengembalikan nol.
  2. "Hanya dalam kasus" (seperti yang ditunjukkan di atas dalam pertanyaan) tidak lengkap dan dengan demikian cacat karena ketidakkonsistenan dalam harapan tentang bagaimana null akan ditangani oleh kode lain di luar / melewati kode p.field = value;.

Pada dasarnya, sepertinya rekan Anda ada di pagar tentang menggunakan pengecualian - meskipun tidak ada pilihan di C # untuk hal-hal seperti itu. ( Jika kita menginginkan kode yang telah teruji dengan baik, kita tidak dapat membuat kode untuk model pengecualian untuk menangani null dan model non-pengecualian menggunakan null-return-values, berdampingan). Mungkin jika Anda beralasan dengan rekan Anda melalui topik-topik ini, mereka akan melihat cahaya!

Erik Eidt
sumber
Anda memukul paku di kepala sehubungan dengan rekan saya berada di pagar tentang pengecualian. Untuk pertanyaan Anda, tidak ada elsekondisi dalam contoh saya. Apa yang akan mereka lakukan hanyalah menghilangkan pekerjaan jika nilainya nol. Dan cek nol itu akan diperbanyak jika instance yang dialokasikan dikembalikan. Baunya seperti kode kesalahan, kan? Saya tidak melihat kasus ini terkait dengan masalah itu, tetapi Anda telah dengan benar menunjukkan bahwa mereka sangat terkait. Terima kasih atas tanggapannya yang komprehensif. Masalah cakupan kode sangat baik bagi saya.
Jason Tyler
Benar, masalahnya kita tidak bisa membuat kode untuk pengecualian dan kode kesalahan di tempat yang sama dan keduanya dapat diuji! Jika mereka benar-benar menginginkan kode kesalahan, mereka dapat membungkus newoperasi dalam coba / tangkap, dan memberikan kode nol dan / atau kesalahan pada kehabisan memori - secara efektif mengubah pengecualian C # untuk OOM menjadi gaya kode kesalahan.
Erik Eidt
1
Kode throw-a-exception bahkan dapat dipersingkat menjadi p = new Object() ?? throw new NullReferenceException();yang mungkin akan membuatnya menjadi kandidat yang lebih jelas untuk optimasi?
wonderra
1
Ada domain tertentu di mana kode mati dianggap sebagai tanggung jawab yang sangat besar sehingga dilarang keras, FWIW.
RubberDuck
8

Mungkin Anda dapat menghindari masalah ini dan cukup beri tahu dia bahwa ada sintaksis c # yang lebih baru yang lebih mudah (dan lakukan pemeriksaan nol untuk Anda!)

Dari pada

Person p = new Person();
if (p != null)
{
    p.Name = "John Smith";
}

Menulis

var p = new Person
{
    Name = "John Smith"
};
John Wu
sumber
2
Sementara itu memecahkan kode contoh, itu tidak secara otomatis berlaku untuk setiap kasus serupa. Gantikan dengan body if block dengan var foo = p.DoSomething()dan Anda akan melihat jawaban Anda tidak berlaku lagi.
Flater
Itu saran yang bagus dan bisa membantu membersihkan beberapa kasus yang sedang saya tangani. Sayangnya contoh saya adalah kasus sederhana dari masalah yang memiliki banyak rasa berbeda, jadi ini tidak akan menghilangkan masalah secara keseluruhan. Tidak ada salahnya!
Jason Tyler
1
@Flater Orang pranoid yang dimaksud kemudian dapat pergi dengan: var foo = p? .DoSomething ();
Eternal21
4

Itu permintaan yang masuk akal, tapi sepertinya ini sudah menjadi pergulatan antara kamu dan dia. Anda mungkin sudah menunjukkannya kepadanya, dia enggan mengubahnya dan sekarang Anda berencana untuk memperbaikinya. Maka Anda mungkin "menang".

Mungkin lebih lancar mengirimkan tautan yang diposting Robert Harvey dan mengatakan, "Saya mengirimi Anda tautan tentang konstruksi objek C # yang mungkin menarik" dan tinggalkan di sana. Tidak persis berbahaya apa yang dia lakukan (hanya sia-sia) dan mudah untuk membersihkan op nanti. Saya katakan: pilih pertempuran Anda. Saya telah berada dalam situasi ini lebih dari sekali dan dalam retrospeksi, seringkali itu tidak sebanding dengan perjuangan. Anda terlalu mudah saling membenci satu sama lain. Ya, Anda benar tetapi sementara itu hanya Anda dan dia, Anda mungkin ingin mempertahankan beberapa kesamaan.

Martin Maat
sumber
Terima kasih atas wawasannya. Poin Anda diambil dengan baik. Saya setuju bahwa pertempuran harus dipilih dengan bijak. Saya sudah mencoba hal "Hei, lihat email ini", tetapi biasanya diabaikan. Saya menjadi ngotot tentang hal ini karena jenis kode "berjaga-jaga" menjadi lebih meresap dalam basis kode kami. Itu membuat semuanya lebih nondeterministic, jadi lebih sulit bagi saya untuk bekerja. Mungkin saya harus mencoba dan menyampaikan poin keseluruhan daripada fokus pada pola pengkodean tertentu?
Jason Tyler
1

Karena pertanyaan teknis Anda telah dijawab (cek nol tidak ada gunanya), saya ingin fokus pada aspek yang berbeda dari pertanyaan Anda - bagaimana menangani situasi seperti itu secara umum: seorang rekan kerja yang tidak mengetahui beberapa fakta dasar tentang alat yang Anda semua gunakan, dan dengan demikian menggunakan alat-alat dengan cara yang kurang optimal.

Dalam beberapa kasus, itu mungkin tidak banyak mempengaruhi Anda. Jika mereka menyelesaikan pekerjaan mereka dan tidak mengganggu pekerjaan Anda, Anda bisa mengabaikan masalahnya.

Dalam kasus lain (misalnya milik Anda), mereka memang mengganggu pekerjaan Anda, setidaknya sampai batas tertentu. Apa yang harus dilakukan tentang hal itu?

Saya pikir itu tergantung pada banyak detail. Misalnya, sudah berapa lama Anda dan rekan kerja Anda bersama perusahaan, bagaimana perusahaan secara umum menangani masalah-masalah seperti itu, seberapa besar masalah itu memengaruhi / mengganggu Anda, seberapa baik Anda bergaul dengan rekan kerja itu, seberapa penting suatu harmonis hubungan dengan rekan kerja Anda kepada Anda, sejauh mana bisa mengemukakan atau meningkatkan masalah memengaruhi hubungan ini, dll.

Pandangan pribadi saya mungkin seperti ini: Saya pikir insinyur harus tahu alat mereka, dan insinyur perangkat lunak harus tahu bahasa pemrograman mereka. Tentu saja, tidak ada yang tahu setiap detail, tetapi konstruktor yang tidak pernah kembali nulladalah ide yang sangat mendasar - semua orang harus mengetahuinya. Ketika saya perlu mengerjakan beberapa kode yang berisi pemeriksaan nol yang tidak ada gunanya, saya cukup menghapusnya. Ketika seorang kolega bertanya kepada saya mengapa saya menghapusnya, saya akan menjelaskan mengapa itu tidak ada gunanya. (Jika perlu, saya juga akan menjelaskan mengapa kode yang sama sekali tidak perlu harus dihapus.) Dalam jangka panjang, ini akan menyebabkan kode yang bebas dari cek nol yang tidak ada gunanya ini.

Ini mungkin terdengar agak sombong, dan mungkin bukan pendekatan yang tepat untuk semua orang. Saya diberitahu bahwa saya sabar dan pandai menjelaskan banyak hal. Mungkin itu sebabnya saya cenderung lolos dengan pendekatan ini tanpa dianggap brengsek. :-)

Anda menyebutkan 'permintaan resmi agar praktik ini dihentikan'. Saya tidak pernah bekerja di sebuah perusahaan yang memiliki aturan formal yang begitu rinci, tetapi di sini ada beberapa pemikiran. Sekali lagi, saya pikir tindakan terbaik Anda tergantung pada detailnya. Jika mengajukan permintaan seperti itu akan membuat rekan kerja Anda terlihat buruk di mata kolega atau manajer, saya mungkin tidak akan melakukannya. Di sisi lain, jika permintaan seperti itu umumnya dianggap sebagai penyambutan yang disambut baik dan yang lain tidak akan menyalahkan rekan kerja Anda karena tidak mengikuti aturan ini sebelumnya, silakan. Tidak ada yang akan dirugikan, semua orang akan lebih baik.


Tetapi jika saya tidak dapat menemukan cara yang baik untuk menangani masalah ini, saya mungkin menggunakan sarkasme dan menambahkan kode seperti ini di mana-mana:

int c = a + b;
if (c == a + b)
{
    // next steps
}
else 
{
    throw new ArithmeticException();
}

Tidak ada gunanya cek nol setelah panggilan konstruktor, tetapi bahkan lebih jelas. Dan tentu saja, saya tidak akan melakukan ini. Kecuali saya sangat kesal dengan rekan kerja saya bahwa saya berencana untuk berhenti. ;-)

jcsahnwaldt mengatakan GoFundMonica
sumber