Cara mengimplementasikan properti pada kelas A yang mengacu pada properti objek anak kelas A

9

Kami memiliki kode ini yang, ketika disederhanakan, terlihat seperti ini:

public class Room
{
    public Client Client { get; set; }

    public long ClientId
    {
        get
        {
            return Client == null ? 0 : Client.Id;
        }
    }
}

public class Client 
{
    public long Id { get; set; }
}

Sekarang kami memiliki tiga sudut pandang.

1) Ini adalah kode yang baik karena Clientproperti harus selalu disetel (yaitu bukan nol) sehingga Client == nulltidak akan pernah terjadi dan nilai Id 0menunjukkan ID palsu pula (ini adalah pendapat penulis kode ;-))

2) Anda tidak dapat bergantung pada penelepon untuk mengetahui bahwa itu 0adalah nilai yang salah untuk Iddan ketika Clientproperti harus selalu disetel Anda harus memasukkan exceptionnilai getketika Clientproperti tersebut bernilai nol.

3) Ketika Clientproperti harus selalu disetel, Anda hanya kembali Client.Iddan membiarkan kode tersebut memberikan NullRefpengecualian ketika Clientproperti tersebut bernilai nol.

Manakah dari ini yang paling benar? Atau apakah ada kemungkinan keempat?

Michel
sumber
5
Bukan jawaban tetapi hanya sebuah catatan: Jangan pernah melemparkan pengecualian dari pengambil properti.
Brandon
3
Untuk menguraikan poin Brandon: stackoverflow.com/a/1488488/569777
MetaFight
1
Tentu: Ide umum ( msdn.microsoft.com/en-us/library/… , stackoverflow.com/questions/1488472/... , antara lain) adalah bahwa properti harus dilakukan sesedikit mungkin, ringan dan harus mewakili kebersihan , keadaan umum objek Anda, dengan pengindeks menjadi sedikit pengecualian. Ini juga perilaku yang tidak terduga untuk melihat pengecualian di jendela arloji untuk properti selama debugging.
Brandon
1
Anda tidak boleh (perlu) menulis kode yang melempar pengambil properti. Itu tidak berarti Anda harus menyembunyikan dan menelan pengecualian yang terjadi karena objek masuk ke kondisi yang tidak didukung.
Ben
4
Mengapa Anda tidak bisa melakukan Room.Client.Id? Mengapa Anda ingin membungkusnya ke dalam Room.ClientId? Jika untuk memeriksa klien maka mengapa bukan sesuatu seperti Room.HasClient {get {return client == null; }} itu lebih lurus ke depan dan masih membiarkan orang melakukan Room.Client normal. Apakah mereka benar-benar membutuhkan id?
James

Jawaban:

25

Baunya seperti Anda harus membatasi jumlah negara bagian Roomkelas Anda .

Fakta bahwa Anda bertanya tentang apa yang harus dilakukan ketika Clientadalah nol adalah petunjuk bahwa Roomruang keadaan terlalu besar.

Sederhananya, saya tidak akan membiarkan Clientproperti dari Roominstance apa pun menjadi nol. Itu berarti kode di dalamnya Roomdapat dengan aman menganggap Clienttidak pernah nol.

Jika karena alasan tertentu di masa depan Clientmenjadi nullmenahan keinginan untuk mendukung negara itu. Melakukan hal itu akan menambah kompleksitas perawatan Anda.

Sebagai gantinya, biarkan kode gagal dan gagal cepat. Bagaimanapun, ini bukan keadaan yang didukung. Jika aplikasi masuk ke keadaan ini, Anda sudah melewati garis tidak dapat kembali. Satu-satunya hal yang masuk akal untuk dilakukan adalah menutup aplikasi.

Ini mungkin terjadi (secara alami) sebagai hasil dari pengecualian referensi nol yang tidak ditangani.

MetaFight
sumber
2
Bagus. Saya menyukai pemikiran "Agar semuanya tetap sederhana, saya tidak akan membiarkan properti Klien dari setiap contoh Kamar menjadi nol." Itu memang lebih baik daripada mempertanyakan apa yang harus dilakukan ketika itu nol
Michel
@Michel jika nanti Anda memutuskan untuk mengubah persyaratan itu, Anda dapat mengganti nullnilainya dengan NullObject(atau lebih tepatnya NullClient) yang kemudian akan tetap bekerja dengan kode Anda yang ada dan memungkinkan Anda untuk menentukan perilaku klien yang tidak ada jika perlu. Pertanyaannya adalah apakah case "no client" adalah bagian dari logika bisnis atau tidak.
null
3
Benar sekali. Jangan menulis satu karakter kode untuk mendukung keadaan yang tidak didukung. Biarkan saja melempar NullRef.
Ben
@ Ben Disagree; Pedoman MS secara eksplisit menyatakan bahwa pengambil properti tidak boleh melempar pengecualian. Dan membiarkan null ref dilemparkan ketika pengecualian yang lebih bermakna bisa dilemparkan malah malas.
Andy
1
@Andy memahami alasan pedoman ini akan membuat Anda tahu kapan itu tidak berlaku.
Ben
21

Hanya beberapa pertimbangan:

a) Mengapa ada pengambil khusus untuk ClientId ketika sudah ada pengambil publik untuk instance Klien itu sendiri? Saya tidak melihat mengapa informasi bahwa ClientId longharus diukir dengan batu di tanda tangan Room.

b) Mengenai pendapat kedua Anda bisa memperkenalkan sebuah konstanta Invalid_Client_Id.

c) Mengenai pendapat satu dan tiga (dan menjadi poin utama saya): Kamar harus selalu memiliki klien? Mungkin hanya semantik tetapi ini kedengarannya tidak benar. Mungkin akan lebih tepat untuk memiliki kelas yang benar-benar terpisah untuk Kamar dan Klien dan kelas lain yang mengikat mereka bersama. Mungkin Pengangkatan, Pemesanan, Hunian? (Ini tergantung pada apa yang sebenarnya Anda lakukan.) Dan pada kelas itu Anda dapat menegakkan batasan "harus memiliki kamar" dan "harus memiliki klien".

VolkerK
sumber
5
+1 untuk "Saya tidak melihat, misalnya, mengapa informasi yang panjang ClientId harus diukir menjadi batu ke tanda tangan Kamar"
Michel
Bahasa Inggris Anda baik-baik saja. ;) Hanya dari membaca jawaban ini, saya tidak akan tahu bahwa bahasa ibu Anda bukan bahasa Inggris tanpa Anda mengatakannya.
jpmc26
Poin B & C bagus; RE: a, ini adalah metode yang mudah, dan fakta bahwa ruangan memiliki referensi ke Klien mungkin hanya detail implementasi (bisa dikatakan bahwa Klien tidak boleh terlihat secara publik)
Andy
17

Saya tidak setuju dengan ketiga pendapat tersebut. Jika Clienttidak pernah bisa null, maka jangan membuatnya menjadi mungkinnull !

  1. Tetapkan nilai Clientdalam konstruktor
  2. Lemparkan sebuah ArgumentNullExceptiondi konstruktor.

Jadi kode Anda akan menjadi seperti:

public class Room
{
    private Client theClient;

    public Room(Client client) {
        if(client == null) throw new ArgumentNullException();
        this.theClient = client;
    }

    public Client Client { 
        get { return theClient; }
        set 
        {
            if (value == null) 
                throw new ArgumentNullException();
            theClient = value;
        }
    }

    public long ClientId
    {
        get
        {
            return theClient.Id;
        }
    }
}
public class Client 
{
    public long Id { get; set; }
}

Ini tidak terkait dengan kode Anda, tetapi Anda mungkin akan lebih baik menggunakan versi abadiRoom dengan membuat theClient readonly, dan kemudian jika klien berubah, membuat ruang baru. Ini akan meningkatkan keamanan utas kode Anda selain keamanan nol dari aspek lain dari jawaban saya. Lihat diskusi ini tentang bisa berubah vs tidak berubah

durron597
sumber
1
Bukankah ini seharusnya ArgumentNullException?
Mathieu Guindon
4
Kode program tidak boleh membuang NullReferenceException, itu dilemparkan oleh .Net VM "ketika ada upaya untuk referensi referensi objek nol" . Anda harus melempar ArgumentNullException, yang dilemparkan "ketika referensi nol (Tidak ada dalam Visual Basic) diteruskan ke metode yang tidak menerimanya sebagai argumen yang valid."
Pharap
2
@Pharap Saya seorang programmer Java yang mencoba menulis kode C #, saya pikir saya akan membuat kesalahan seperti itu.
durron597
@ durron597 Saya seharusnya menebak bahwa dari penggunaan istilah 'final' (dalam hal ini kata kunci C # yang sesuai adalah readonly). Saya baru saja mengedit tetapi saya merasa komentar akan lebih menjelaskan mengapa (untuk pembaca masa depan). Jika Anda belum memberikan jawaban ini, saya akan memberikan solusi yang sama persis. Kekekalan juga merupakan ide yang bagus, tetapi itu tidak selalu semudah yang diharapkan.
Pharap
2
Properti umumnya tidak boleh dilemparkan pengecualian; alih-alih setter yang melempar ArgumentNullException, berikan metode SetClient sebagai gantinya. Seseorang memposting tautan ke jawaban mengenai hal ini pada pertanyaan.
Andy
5

Opsi kedua dan ketiga harus dihindari - rajin dan rajin giat memukul si penelepon dengan pengecualian mereka tidak memiliki kendali atas.

Anda harus memutuskan apakah Klien dapat menjadi nol. Jika demikian, Anda harus menyediakan cara bagi pemanggil untuk memeriksa apakah itu nol sebelum mengaksesnya (misalnya, bool properti ClientIsNull bool).

Jika Anda memutuskan bahwa Klien tidak akan pernah menjadi nol, maka jadikan parameter yang diperlukan untuk konstruktor dan berikan pengecualian di sana jika nol dilewatkan.

Akhirnya, opsi pertama juga berisi kode bau. Anda harus membiarkan Klien berurusan dengan properti ID-nya sendiri. Sepertinya berlebihan untuk membuat kode rajin dan giat dalam kelas wadah yang hanya memanggil rajin dan rajin pada kelas yang ada. Hanya mengekspos Klien sebagai properti (jika tidak, Anda akan akhirnya menduplikasi semua yang sudah ditawarkan oleh Klien ).

long clientId = room.Client.Id;

Jika Klien dapat menjadi nol, maka Anda setidaknya akan memberikan tanggung jawab kepada penelepon:

if (room.Client != null){
    long clientId = room.Client.Id;
    /* other code follows... */
}
Kent A.
sumber
1
Saya pikir "Anda akan menduplikasi segala sesuatu yang sudah ditawarkan klien" harus dicetak tebal atau miring.
Pharap
2

Jika properti Klien nol adalah keadaan yang didukung, pertimbangkan untuk menggunakan NullObject .

Tetapi kemungkinan besar ini adalah keadaan yang luar biasa, jadi Anda harus membuatnya tidak mungkin (atau tidak terlalu nyaman) untuk berakhir dengan nol Client:

public Client Client { get; private set; }

public Room(Client client)
{
    Client = client;
}

public void SetClient(Client client)
{
    if (client == null) throw new ArgumentNullException();
    Client = client;
}

Namun jika tidak didukung, jangan buang waktu untuk solusi setengah matang. Pada kasus ini:

return Client == null ? 0 : Client.Id;

Anda telah 'memecahkan' NullReferenceException di sini tetapi dengan biaya yang besar! Ini akan memaksa semua penelepon Room.ClientIduntuk memeriksa 0:

var aRoom = new Room(aClient);
var clientId = aRoom.ClientId;
if (clientId == 0) RestartRoombookingWizard();
else ContinueRoombooking(clientid);

Bug aneh dapat berganti dengan devs lain (atau diri Anda sendiri!) Lupa untuk memeriksa nilai kembali "ErrorCode" di beberapa titik waktu.
Mainkan dengan aman dan gagal dengan cepat. Biarkan NullReferenceException dilempar dan "anggun" tangkap di tempat yang lebih tinggi di tumpukan panggilan alih-alih memungkinkan penelepon mengacaukan segalanya ...

Pada catatan yang berbeda, jika Anda begitu tertarik untuk mengekspos Clientdan juga ClientIdmemikirkan TellDontAsk . Jika Anda bertanya terlalu banyak dari objek alih-alih memberi tahu mereka apa yang ingin Anda capai, Anda dapat mengakhiri menyatukan semuanya, membuat perubahan lebih sulit nantinya.

Laoujin
sumber
Itu NotSupportedExceptiondisalahgunakan, Anda harus menggunakan sebagai ArgumentNullExceptiongantinya.
Pharap
1

Pada c # 6.0, yang sekarang dirilis, Anda harus melakukan ini,

public class Room
{
    public Room(Client client)
    {
        this.Client = client;
    }

    public Client Client { get; }
}

public class Client
{
    public Client(long id)
    {
        this.Id = id;
    }

    public long Id { get; }
}

Mengangkat properti Clientke Roommerupakan pelanggaran nyata enkapsulasi dan prinsip KERING.

Jika Anda perlu mengakses Ida Clientdari yang Roomdapat Anda lakukan,

var clientId = room.Client?.Id;

Perhatikan penggunaan Operator Bersyarat Null , clientIdakan menjadi long?, jika Clientada null, clientIdakan null, jika tidak clientIdakan memiliki nilai Client.Id.

Jodrell
sumber
tetapi jenis clientidadalah nullable lama itu?
Michel
@Michel yah, jika sebuah ruangan harus memiliki seorang Klien, beri tanda centang nol di ctor. Maka var clientId = room.Client.Idkeduanya akan aman dan long.
Jodrell