Jika fungsi harus melakukan pemeriksaan nol sebelum melakukan perilaku yang dimaksud, apakah desain yang buruk ini?

Jadi saya tidak tahu apakah ini desain kode yang baik atau buruk, jadi saya pikir saya sebaiknya bertanya.

Saya sering membuat metode yang melakukan pengolahan data yang melibatkan kelas dan saya sering melakukan banyak pemeriksaan dalam metode untuk memastikan saya tidak mendapatkan referensi nol atau kesalahan lain sebelumnya.

Untuk contoh yang sangat mendasar:

// fields and properties
private Entity _someEntity;
public Entity SomeEntity => _someEntity;

public void AssignEntity(Entity entity){
    _someEntity = entity;
}
public void SetName(string name)
{
    if (_someEntity == null) return; //check to avoid null ref
    _someEntity.Name = name;
    label.SetText(_someEntity.Name);
}

Jadi seperti yang Anda lihat, saya memeriksa null setiap kali. Tetapi haruskah metode ini tidak memiliki pemeriksaan ini?

Misalnya, kode eksternal harus membersihkan data sebelumnya sehingga metode tidak perlu divalidasi seperti di bawah ini:

 if(entity != null) // this makes the null checks redundant in the methods
 {
     Manager.AssignEntity(entity);
     Manager.SetName("Test");
 }

Singkatnya, metode harus "memvalidasi data" dan kemudian melakukan pemrosesan pada data, atau harus dijamin sebelum Anda memanggil metode, dan jika Anda gagal untuk memvalidasi sebelum memanggil metode, itu harus menimbulkan kesalahan (atau menangkap kesalahan)?

Masalah dengan contoh dasar Anda bukan cek nol, itu gagal diam .

Null pointer / kesalahan referensi, lebih sering daripada tidak, adalah kesalahan programmer. Kesalahan pemrogram sering kali paling baik diatasi dengan gagal dengan segera dan keras. Anda memiliki tiga cara umum untuk mengatasi masalah dalam situasi ini:

1. Jangan repot-repot memeriksa, dan biarkan saja runtime membuang pengecualian nullpointer.
2. Periksa, dan lempar pengecualian dengan pesan lebih baik dari pesan NPE dasar.

Solusi ketiga adalah lebih banyak pekerjaan tetapi jauh lebih kuat:

3. Struktur kelas Anda sedemikian rupa sehingga hampir tidak mungkin untuk _someEntityberada dalam keadaan tidak valid. Salah satu cara untuk melakukannya adalah dengan menyingkirkan AssignEntitydan memerlukannya sebagai parameter untuk instantiasi. Teknik Injeksi Ketergantungan Lainnya dapat bermanfaat juga.

Dalam beberapa situasi, masuk akal untuk memeriksa semua argumen fungsi untuk validitas sebelum pekerjaan apa pun yang Anda lakukan, dan dalam situasi lain masuk akal untuk memberi tahu pemanggil bahwa mereka bertanggung jawab untuk memastikan input mereka valid, dan tidak memeriksa. Di mana ujung spektrum yang Anda gunakan akan bergantung pada domain masalah Anda. Opsi ketiga memiliki manfaat yang signifikan karena Anda dapat, sampai batas tertentu, membuat kompiler memastikan bahwa pemanggil melakukan semuanya dengan benar.

Jika opsi ketiga bukan opsi, maka menurut saya itu tidak masalah asalkan tidak diam-diam gagal. Jika tidak memeriksa null menyebabkan program meledak secara instan, baik, tetapi jika malah diam-diam merusak data yang menyebabkan masalah di jalan, maka yang terbaik adalah memeriksa dan menanganinya saat itu juga.

Karena _someEntitydapat dimodifikasi pada tahap apa pun, maka masuk akal untuk mengujinya setiap kali SetNamedipanggil. Bagaimanapun, itu bisa saja berubah sejak terakhir kali metode itu dipanggil. Namun ketahuilah bahwa kode SetNameini tidak aman untuk digunakan, sehingga Anda dapat melakukan pemeriksaan tersebut dalam satu utas, telah _someEntityditetapkan nulloleh utas lainnya dan kemudian kode tersebut akan muntah NullReferenceException.

Karena itu, salah satu pendekatan untuk masalah ini adalah menjadi lebih defensif dan melakukan salah satu dari yang berikut:

1. membuat salinan lokal _someEntitydan referensi yang menyalin melalui metode ini,
2. gunakan kunci di sekitar _someEntityuntuk memastikan itu tidak berubah saat metode dijalankan,
3. gunakan a try/catchdan lakukan tindakan yang sama pada apa pun NullReferenceExceptionyang terjadi dalam metode seperti yang akan Anda lakukan dalam pemeriksaan nol awal (dalam hal ini, noptindakan).

Tetapi yang harus Anda lakukan adalah berhenti dan tanyakan pada diri sendiri pertanyaan: apakah Anda benar-benar perlu untuk _someEntityditimpa? Mengapa tidak mengaturnya sekali saja, melalui konstruktor. Dengan begitu, Anda hanya perlu melakukan itu nol periksa sekali:

private readonly Entity _someEntity;

public Constructor(Entity someEntity)
{
    if (someEntity == null) throw new ArgumentNullException(nameof(someEntity));
    _someEntity = someEntity;
}

public void SetName(string name)
{
    _someEntity.Name = name;
    label.SetText(_someEntity.Name);
}

Jika memungkinkan, ini adalah pendekatan yang saya sarankan dalam situasi seperti itu. Jika Anda tidak bisa memperhatikan keamanan utas saat memilih cara menangani kemungkinan nol.

Sebagai komentar bonus, saya merasa kode Anda aneh dan dapat ditingkatkan. Semua:

private Entity _someEntity;
public Entity SomeEntity => _someEntity;

public void AssignEntity(Entity entity){
    _someEntity = entity;
}

dapat diganti dengan:

public Entity SomeEntity { get; set; }

Yang memiliki fungsi yang sama, simpan untuk dapat mengatur bidang melalui setter properti, daripada metode dengan nama yang berbeda.

Tetapi haruskah metode ini tidak memiliki pemeriksaan ini?

Ini adalah Anda pilihan.

Dengan membuat publicmetode, Anda menawarkan kesempatan kepada publik untuk menyebutnya. Ini selalu disertai dengan kontrak implisit tentang cara memanggil metode itu dan apa yang diharapkan ketika melakukannya. Kontrak ini mungkin (atau mungkin tidak) termasuk " yeah, uhhm, jika Anda lulus nullsebagai nilai parameter, itu akan meledak tepat di wajah Anda. ". Bagian lain dari kontrak itu mungkin " oh, BTW: setiap kali dua utas melaksanakan metode ini pada saat yang sama, seekor anak kucing mati ".

Jenis kontrak apa yang ingin Anda rancang terserah Anda. Yang penting adalah untuk

• buat API yang bermanfaat dan konsisten dan

• untuk mendokumentasikannya dengan baik, terutama untuk kasus tepi tersebut.

Apa kemungkinan kasus bagaimana metode Anda dipanggil?

Jawaban lainnya baik; Saya ingin memperluasnya dengan membuat komentar tentang pengubah aksesibilitas. Pertama, apa yang harus dilakukan jika nulltidak pernah valid:

• metode umum dan terlindungi adalah metode di mana Anda tidak mengontrol penelepon. Mereka harus membuang ketika melewati nol. Dengan begitu Anda melatih penelepon Anda untuk tidak pernah memberi Anda nol, karena mereka ingin program mereka tidak crash.

• intern dan swasta metode adalah mereka di mana Anda tidak mengontrol pemanggil. Mereka harus menegaskan ketika lulus nol; mereka mungkin akan crash nanti, tetapi setidaknya Anda mendapatkan pernyataan pertama, dan kesempatan untuk masuk ke debugger. Sekali lagi, Anda ingin melatih penelepon Anda untuk memanggil Anda dengan benar dengan membuatnya sakit ketika mereka tidak.

Sekarang, apa yang Anda lakukan jika mungkin valid untuk null dilewatkan? Saya akan menghindari situasi ini dengan pola objek nol . Yaitu: membuat instance khusus dari jenis dan mengharuskan itu digunakan setiap saat bahwa objek yang tidak valid diperlukan . Sekarang Anda kembali ke dunia menyenangkan melempar / menegaskan setiap penggunaan nol.

Misalnya, Anda tidak ingin berada dalam situasi ini:

class Person { ... }
...
class ExpenseReport { 
  public void SubmitReport(Person approver) { 
    if (approver == null) ... do something ...

dan di situs panggilan:

// I guess this works but I don't know what it means
expenses.SubmitReport(null); 

Karena (1) Anda melatih penelepon Anda bahwa null adalah nilai yang baik, dan (2) tidak jelas apa arti semantik dari nilai itu. Sebaliknya, lakukan ini:

class ExpenseReport { 
  public static readonly Person ApproverNotRequired = new Person();
  public static readonly Person ApproverUnknown = new Person();
  ...
  public void SubmitReport(Person approver) { 
    if (approver == null) throw ...
    if (approver == ApproverNotRequired) ... do something ...
    if (approver == ApproverUnknown) ... do something ...

Dan sekarang situs panggilan terlihat seperti ini:

expenses.SubmitReport(ExpenseReport.ApproverNotRequired);

dan sekarang pembaca kode tahu apa artinya.

Jawaban lain menunjukkan bahwa kode Anda dapat dibersihkan hingga tidak memerlukan pemeriksaan nol di mana Anda memilikinya, namun, untuk jawaban umum tentang apa yang dapat dilakukan pemeriksaan nol berguna untuk mempertimbangkan kode sampel berikut:

public static class Foo {
    public static void Frob(Bar a, Bar b) {
        if (a == null) throw new ArgumentNullException(nameof(a));
        if (b == null) throw new ArgumentNullException(nameof(b));

        a.Something = b.Something;
    }
}

Ini memberi Anda keuntungan untuk pemeliharaan. Jika pernah terjadi kerusakan dalam kode Anda di mana sesuatu melewati objek nol ke metode Anda akan mendapatkan informasi yang berguna dari metode yang parameternya nol. Tanpa cek, Anda akan mendapatkan NullReferenceException di telepon:

a.Something = b.Something;

Dan (mengingat bahwa properti Something adalah tipe nilai) a atau b bisa menjadi nol dan Anda tidak akan memiliki cara untuk mengetahui mana dari jejak stack. Dengan cek, Anda akan tahu persis objek mana yang nol, yang bisa sangat berguna ketika mencoba membongkar cacat.

Saya akan perhatikan bahwa pola dalam kode asli Anda:

if (x == null) return;

Dapat memiliki kegunaannya dalam keadaan tertentu, juga dapat (mungkin lebih sering) memberi Anda cara yang sangat baik untuk menutupi masalah mendasar dalam basis kode Anda.

Tidak tidak sama sekali, tapi itu tergantung.

Anda hanya melakukan pemeriksaan referensi nol jika Anda ingin bereaksi.

Jika Anda melakukan pemeriksaan referensi nol dan melempar pengecualian yang diperiksa , Anda memaksa pengguna metode untuk bereaksi dan membiarkannya pulih. Program masih berjalan seperti yang diharapkan.

Jika Anda tidak melakukan pemeriksaan referensi nol (atau melemparkan pengecualian yang tidak dicentang), itu mungkin membuang yang tidak dicentang NullReferenceException, yang biasanya tidak ditangani oleh pengguna metode ini dan bahkan mungkin mematikan aplikasi. Ini berarti bahwa fungsinya jelas-jelas disalahpahami dan oleh karena itu aplikasinya salah.

Sesuatu dari ekstensi ke jawaban @ null, tetapi dalam pengalaman saya, lakukan pemeriksaan nol, apa pun yang terjadi.

Pemrograman defensif yang baik adalah sikap yang Anda kode rutin saat ini dalam isolasi, dengan asumsi bahwa seluruh program mencoba untuk menghentikannya. Ketika Anda sedang menulis kode kritis misi di mana kegagalan bukanlah suatu pilihan, ini adalah satu-satunya cara untuk pergi.

Sekarang, bagaimana Anda benar-benar berurusan dengan suatu nullnilai, itu sangat tergantung pada kasus penggunaan Anda.

Jika nullmerupakan nilai yang dapat diterima, misalnya parameter kedua hingga kelima ke rutin MSVC _splitpath (), maka secara diam-diam berurusan dengannya adalah perilaku yang benar.

Jika nullseharusnya tidak pernah terjadi ketika memanggil rutin yang dimaksud, yaitu dalam kasus OP, kontrak API AssignEntity()harus telah dipanggil dengan valid, Entitylalu melemparkan pengecualian, atau gagal memastikan Anda membuat banyak suara dalam proses.

Jangan pernah khawatir tentang biaya cek nol, mereka sangat murah jika terjadi, dan dengan kompiler modern, analisis kode statis dapat dan akan menghilangkannya sepenuhnya jika kompiler menentukan bahwa itu tidak akan terjadi.