Penerapan Prinsip Tanggung Jawab Tunggal

40

Baru-baru ini saya datang dengan masalah arsitektur yang tampaknya sepele. Saya memiliki repositori sederhana dalam kode saya yang dipanggil seperti ini (kode dalam C #):

var user = /* create user somehow */;
_userRepository.Add(user);
/* do some other stuff*/
_userRepository.SaveChanges();

SaveChanges adalah pembungkus sederhana yang melakukan perubahan pada basis data:

void SaveChanges()
{
    _dataContext.SaveChanges();
    _logger.Log("User DB updated: " + someImportantInfo);
}

Kemudian, setelah beberapa waktu, saya perlu menerapkan logika baru yang akan mengirim pemberitahuan email setiap kali pengguna dibuat dalam sistem. Karena ada banyak panggilan ke _userRepository.Add()dan di SaveChangessekitar sistem, saya memutuskan untuk memperbarui SaveChangesseperti ini:

void SaveChanges()
{
    _dataContext.SaveChanges();
    _logger.Log("User DB updated: " + someImportantInfo);
    foreach (var newUser in dataContext.GetAddedUsers())
    {
       _eventService.RaiseEvent(new UserCreatedEvent(newUser ))
    }
}

Dengan cara ini, kode eksternal dapat berlangganan UserCreatedEvent dan menangani logika bisnis yang diperlukan yang akan mengirim pemberitahuan.

Tetapi ditunjukkan kepada saya bahwa modifikasi saya SaveChangesmelanggar prinsip Tanggung Jawab Tunggal, dan itu SaveChangesseharusnya menyelamatkan dan tidak memecat suatu peristiwa.

Apakah ini poin yang valid? Sepertinya saya bahwa memunculkan suatu acara di sini pada dasarnya adalah hal yang sama dengan logging: hanya menambahkan beberapa fungsi sisi ke fungsi. Dan SRP tidak melarang Anda untuk menggunakan aktivitas logging atau menembak di fungsi Anda, itu hanya mengatakan bahwa logika tersebut harus dienkapsulasi di kelas lain, dan itu boleh untuk repositori untuk memanggil kelas-kelas lain ini.

Andre Borges
sumber
22
Retort Anda adalah: "Oke, jadi bagaimana Anda akan menulisnya sehingga tidak melanggar SRP tetapi masih memungkinkan satu titik modifikasi?"
Robert Harvey
43
Pengamatan saya adalah bahwa meningkatkan acara tidak menambah tanggung jawab tambahan. Bahkan, justru sebaliknya: ia mendelegasikan tanggung jawab di tempat lain.
Robert Harvey
Saya pikir rekan kerja Anda benar, tetapi pertanyaan Anda valid dan bermanfaat, jadi terangkat!
Andres F.
16
Tidak ada definisi definitif dari Satu Tanggung Jawab. Orang yang menyatakan bahwa itu melanggar SRP benar menggunakan definisi pribadi mereka dan Anda benar menggunakan definisi Anda. Saya pikir desain Anda baik-baik saja dengan peringatan bahwa acara ini bukan satu kali dimana fungsi serupa lainnya dilakukan dengan cara yang berbeda. Konsistensi jauh, jauh, jauh lebih penting untuk diperhatikan daripada beberapa pedoman yang tidak jelas seperti SRP yang dibawa ke ekstrem berakhir dengan banyak kelas yang sangat mudah dipahami yang tidak ada yang tahu bagaimana membuat kerja dalam suatu sistem.
Dunk

Jawaban:

14

Ya, ini bisa menjadi persyaratan yang valid untuk memiliki repositori yang memecat peristiwa tertentu pada tindakan tertentu seperti Addatau SaveChanges- dan saya tidak akan mempertanyakan ini (seperti beberapa jawaban lain) hanya karena contoh spesifik Anda menambahkan pengguna dan mengirim email mungkin terlihat seperti sedikit dibuat-buat. Berikut ini, mari kita asumsikan persyaratan ini dibenarkan secara sempurna dalam konteks sistem Anda.

Jadi ya , pengkodean mekanisme acara serta pencatatan serta penghematan menjadi satu metode melanggar SRP . Untuk banyak kasus, ini mungkin merupakan pelanggaran yang dapat diterima, terutama ketika tidak seorang pun ingin mendistribusikan tanggung jawab pemeliharaan "simpan perubahan" dan "angkat acara" ke tim / pengelola yang berbeda. Tetapi mari kita asumsikan suatu hari seseorang ingin melakukan hal ini, dapatkah itu diselesaikan dengan cara yang sederhana, mungkin dengan memasukkan kode kekhawatiran tersebut ke perpustakaan kelas yang berbeda?

Solusi untuk ini adalah membiarkan repositori asli Anda tetap bertanggung jawab untuk melakukan perubahan pada database, tidak ada yang lain, dan membuat repositori proxy yang memiliki antarmuka publik yang persis sama, menggunakan kembali repositori asli dan menambahkan mekanisme acara tambahan ke metode.

// In EventFiringUserRepo:
public void SaveChanges()
{
  _basicRepo.SaveChanges();
   FireEventsForNewlyAddedUsers();
}

private void FireEventsForNewlyAddedUsers()
{
  foreach (var newUser in _basicRepo.DataContext.GetAddedUsers())
  {
     _eventService.RaiseEvent(new UserCreatedEvent(newUser))
  }
}

Anda dapat memanggil kelas proxy a NotifyingRepositoryatau ObservableRepositoryjika Anda suka, di sepanjang jawaban @ Peter yang sangat dipilih (yang sebenarnya tidak memberi tahu bagaimana mengatasi pelanggaran SRP, hanya mengatakan bahwa pelanggarannya ok).

Kelas repositori baru dan lama harus berasal dari antarmuka umum, seperti yang ditunjukkan pada deskripsi pola Proxy klasik .

Kemudian, dalam kode asli Anda, inisialisasi _userRepositorydengan objek dari EventFiringUserRepokelas baru . Dengan begitu, Anda menjaga repositori asli terpisah dari mekanisme acara. Jika diperlukan, Anda dapat meminta repositori pemicu acara dan repositori asli berdampingan dan membiarkan penelepon memutuskan apakah mereka menggunakan yang pertama atau yang terakhir.

Untuk mengatasi satu kekhawatiran yang disebutkan dalam komentar: bukankah itu mengarah pada proxy di atas proxy di atas proxy, dan sebagainya? Sebenarnya, menambahkan mekanisme acara menciptakan dasar untuk menambahkan persyaratan lebih lanjut dari jenis "kirim email" dengan hanya berlangganan acara tersebut, jadi tetap berpegang pada SRP dengan persyaratan tersebut juga, tanpa proxy tambahan. Tetapi satu hal yang harus ditambahkan sekali di sini adalah mekanik acara itu sendiri.

Jika pemisahan seperti ini benar-benar layak dalam konteks sistem Anda adalah sesuatu yang Anda dan pengulas Anda harus memutuskan sendiri. Saya mungkin tidak akan memisahkan pencatatan dari kode asli, juga tidak dengan menggunakan proksi lain tidak dengan menambahkan logger ke acara pendengar, meskipun itu mungkin.

Doc Brown
sumber
3
Selain jawaban ini. Ada alternatif untuk proksi, seperti AOP .
Laiv
1
Saya pikir Anda melewatkan intinya, ini bukan berarti meningkatkan acara merusak SRP-nya, bahwa meningkatkan acara hanya untuk pengguna "Baru" mengharuskan repo bertanggung jawab untuk mengetahui apa yang merupakan pengguna "Baru" daripada "Baru Ditambahkan ke Saya "pengguna
Ewan
@ Ewan: tolong baca lagi pertanyaannya. Itu tentang tempat dalam kode yang melakukan tindakan tertentu yang perlu digabungkan dengan tindakan lain di luar tanggung jawab objek itu. Dan menempatkan aksi dan acara peningkatan di satu tempat dipertanyakan oleh peer reviewer sebagai melanggar SRP. Contoh "menyelamatkan pengguna baru" hanya untuk tujuan demonstrasi, sebut contoh yang dibuat jika Anda suka, tapi itu IMHO bukan inti dari pertanyaan.
Doc Brown
2
Ini adalah jawaban IMO terbaik / benar. Tidak hanya mempertahankan SRP, tetapi juga mempertahankan prinsip Terbuka / Tertutup. Dan pikirkan semua tes otomatis yang dapat dipecah di dalam kelas. Memodifikasi tes yang ada ketika fungsionalitas baru ditambahkan adalah bau besar.
Keith Payne
1
Bagaimana cara kerja solusi ini jika ada transaksi yang sedang berlangsung? Ketika itu terjadi, SaveChanges()sebenarnya tidak membuat catatan database, dan itu bisa berakhir dibatalkan. Sepertinya Anda harus mengganti AcceptAllChangesatau berlangganan acara TransactionCompleted.
John Wu
29

Mengirim pemberitahuan bahwa penyimpanan data persisten berubah tampaknya merupakan hal yang masuk akal untuk dilakukan saat menyimpan.

Tentu saja Anda tidak boleh memperlakukan Tambahkan sebagai kasus khusus - Anda harus memecat acara untuk Ubah dan Hapus juga. Ini adalah perlakuan khusus dari kasus "Add" yang berbau, memaksa pembaca untuk menjelaskan mengapa baunya, dan akhirnya membuat beberapa pembaca kode menyimpulkan bahwa itu harus melanggar SRP.

Repositori "notifikasi" yang dapat ditanyakan, diubah, dan memancarkan kejadian pada perubahan, adalah objek yang sangat normal. Anda dapat mengharapkan untuk menemukan beberapa variasi daripadanya di hampir semua proyek berukuran layak.


Tetapi apakah repositori "memberitahukan" sebenarnya yang Anda butuhkan? Anda menyebutkan C #: Banyak orang akan setuju bahwa menggunakan System.Collections.ObjectModel.ObservableCollection<>alih - alih System.Collections.Generic.List<>ketika yang terakhir adalah semua yang Anda butuhkan adalah semua jenis yang buruk dan salah, tetapi sedikit yang akan langsung menunjuk ke SRP.

Apa yang Anda lakukan sekarang adalah menukar UserList _userRepositorydengan ObservableUserCollection _userRepository. Jika itu tindakan terbaik atau tidak tergantung pada aplikasi. Tetapi sementara itu tidak diragukan lagi membuat yang _userRepositoryjauh lebih ringan, menurut pendapat saya yang sederhana itu tidak melanggar SRP.

Peter
sumber
Masalah dengan menggunakan ObservableCollectionuntuk kasus ini adalah, ia memicu peristiwa yang setara bukan pada saat dihubungi SaveChanges, tetapi pada saat dihubungi Add, yang akan mengarah pada perilaku yang sangat berbeda dari yang ditunjukkan dalam contoh. Lihat jawaban saya bagaimana menjaga repo asli tetap ringan, dan tetap berpegang teguh pada SRP dengan menjaga semantik tetap utuh.
Doc Brown
@DocBrown Saya meminta kelas yang diketahui ObservableCollection<>dan List<>untuk perbandingan dan konteks. Saya tidak bermaksud merekomendasikan penggunaan kelas aktual untuk implementasi internal atau antarmuka eksternal.
Peter
Ok, tetapi bahkan jika OP akan menambahkan acara ke "Ubah" dan "Hapus" (yang saya pikir OP tinggalkan untuk menjaga pertanyaan tetap singkat, demi kesederhanaan), saya pikir resensi dapat dengan mudah sampai pada kesimpulan dari pelanggaran SRP. Ini mungkin yang dapat diterima, tetapi tidak ada yang tidak bisa diselesaikan jika diperlukan.
Doc Brown
16

Ya, itu merupakan pelanggaran prinsip tanggung jawab tunggal dan poin yang valid.

Desain yang lebih baik adalah memiliki proses terpisah untuk mengambil 'pengguna baru' dari repositori dan mengirim email. Melacak pengguna yang telah dikirimi email, kegagalan, mengirim ulang, dll., Dll.

Dengan cara ini Anda dapat menangani kesalahan, macet dan sejenisnya serta menghindari repositori Anda mengambil setiap persyaratan yang memiliki gagasan bahwa peristiwa terjadi "ketika sesuatu dikomit ke database".

Repositori tidak tahu bahwa pengguna yang Anda tambahkan adalah pengguna baru. Tanggung jawabnya hanya menyimpan pengguna.

Mungkin perlu diperluas pada komentar di bawah ini.

Repositori tidak tahu bahwa pengguna yang Anda tambahkan adalah pengguna baru - ya itu, ia memiliki metode yang disebut Tambah. Semantiknya menyiratkan bahwa semua pengguna yang ditambahkan adalah pengguna baru. Gabungkan semua argumen yang diteruskan ke Tambah sebelum memanggil Simpan - dan Anda mendapatkan semua pengguna baru

Salah. Anda menyatukan "Ditambahkan ke Repositori" dan "Baru".

"Ditambahkan ke Repositori" berarti apa yang dikatakannya. Saya dapat menambah dan menghapus dan menambahkan kembali pengguna ke berbagai repositori.

"Baru" adalah keadaan pengguna yang ditentukan oleh aturan bisnis.

Saat ini aturan bisnis mungkin "Baru == baru saja ditambahkan ke repositori", tetapi itu tidak berarti itu bukan tanggung jawab terpisah untuk mengetahui dan menerapkan aturan itu.

Anda harus berhati-hati untuk menghindari pemikiran seperti ini. Anda akan memiliki proses kasus tepi yang menambahkan pengguna bukan baru ke repositori dan ketika Anda mengirim email kepada mereka semua bisnis akan mengatakan "Tentu saja mereka bukan pengguna 'baru'! Aturan sebenarnya adalah X"

Jawaban ini benar-benar tidak ada artinya IMHO: repo persis satu tempat pusat dalam kode yang tahu kapan pengguna baru ditambahkan

Salah. Untuk alasan di atas, ditambah lagi itu bukan lokasi pusat kecuali Anda benar-benar memasukkan kode pengiriman email di kelas daripada hanya meningkatkan acara.

Anda akan memiliki aplikasi yang menggunakan kelas repositori, tetapi tidak memiliki kode untuk mengirim email. Ketika Anda menambahkan pengguna di aplikasi-aplikasi itu, email tidak akan dikirim.

Ewan
sumber
11
Repositori tidak tahu bahwa pengguna yang Anda tambahkan adalah pengguna baru - ya itu, ia memiliki metode yang disebut Add. Semantiknya menyiratkan bahwa semua pengguna yang ditambahkan adalah pengguna baru. Gabungkan semua argumen yang diteruskan Addsebelum menelepon Save- dan Anda mendapatkan semua pengguna baru.
Andre Borges
Saya suka saran ini. Namun, pragmatisme menang atas kemurnian. Bergantung pada situasinya, menambahkan lapisan arsitektur yang sama sekali baru ke aplikasi yang ada bisa sulit untuk dibenarkan jika semua yang perlu Anda lakukan adalah benar-benar mengirim satu email ketika pengguna ditambahkan.
Alexander
Namun acara tersebut tidak mengatakan pengguna menambahkan. Dikatakan pengguna dibuat. Jika kami mempertimbangkan memberi nama dengan benar dan kami setuju dengan perbedaan semantik antara tambah dan buat, maka acara di cuplikan salah nama atau salah tempat. Saya tidak berpikir resensi memiliki menentang notyfing repositori. Mungkin itu mengkhawatirkan jenis acara dan efek sampingnya.
Laiv
7
@And Baru dalam repo, tetapi tidak harus "baru" dalam pengertian bisnis. ini adalah perpaduan dua gagasan ini yang menyembunyikan tanggung jawab ekstra dari pandangan pertama. Saya mungkin mengimpor satu ton pengguna lama ke repositori baru saya, atau menghapus dan menambahkan kembali pengguna dll. Akan ada aturan bisnis tentang apa "pengguna baru" di luar "telah ditambahkan ke dB"
Ewan
1
Moderator Catatan: Jawaban Anda bukan wawancara jurnalistik. Jika Anda memiliki pengeditan, gabungkan secara alami ke dalam jawaban Anda tanpa membuat seluruh efek "berita terkini". Kami bukan forum diskusi.
Robert Harvey
7

Apakah ini poin yang valid?

Ya itu, meskipun itu sangat tergantung pada struktur kode Anda. Saya tidak memiliki konteks penuh sehingga saya akan mencoba untuk berbicara secara umum.

Tampaknya bagi saya bahwa meningkatkan acara di sini pada dasarnya adalah hal yang sama dengan pencatatan: hanya menambahkan beberapa fungsi samping ke fungsi tersebut.

Sama sekali tidak. Logging bukan bagian dari aliran bisnis, itu dapat dinonaktifkan, tidak boleh menyebabkan efek samping (bisnis) dan tidak boleh mempengaruhi keadaan dan kesehatan aplikasi Anda dengan cara apa pun, bahkan jika Anda karena suatu alasan tidak dapat login apapun lagi. Sekarang bandingkan dengan logika yang Anda tambahkan.

Dan SRP tidak melarang Anda untuk menggunakan aktivitas logging atau menembak di fungsi Anda, itu hanya mengatakan bahwa logika tersebut harus dienkapsulasi di kelas lain, dan itu boleh untuk repositori untuk memanggil kelas-kelas lain ini.

SRP bekerja bersama dengan ISP (S dan I dalam SOLID). Anda berakhir dengan banyak kelas dan metode yang melakukan hal yang sangat spesifik dan tidak ada yang lain. Mereka sangat fokus, sangat mudah untuk memperbarui atau mengganti, dan secara umum mudah untuk menguji. Tentu saja dalam praktiknya Anda juga akan memiliki beberapa kelas yang lebih besar yang berhubungan dengan orkestrasi: mereka akan memiliki sejumlah dependensi, dan mereka tidak akan fokus pada aksi yang dikabutkan, tetapi pada aksi bisnis, yang mungkin memerlukan beberapa langkah. Selama konteks bisnisnya jelas, mereka juga bisa disebut tanggung jawab tunggal, tetapi ketika Anda mengatakan dengan benar, ketika kode bertambah, Anda mungkin ingin mengabstraksi sebagian ke dalam kelas / antarmuka baru.

Sekarang kembali ke contoh khusus Anda. Jika Anda benar-benar harus mengirim pemberitahuan setiap kali pengguna dibuat dan bahkan mungkin melakukan tindakan khusus lainnya, maka Anda dapat membuat layanan terpisah yang merangkum persyaratan ini, seperti UserCreationService, yang memaparkan satu metode Add(user),, yang menangani kedua penyimpanan (panggilan ke repositori Anda) dan pemberitahuan sebagai satu aksi bisnis. Atau lakukan di cuplikan asli Anda, setelahnya_userRepository.SaveChanges();

async
sumber
2
Penebangan bukan bagian dari aliran bisnis - bagaimana hal ini relevan dalam konteks SRP? Jika tujuan acara saya adalah mengirim data pengguna baru ke Google Analytics - maka menonaktifkannya akan memiliki efek bisnis yang sama dengan menonaktifkan pencatatan: tidak penting, tetapi cukup mengecewakan. Apa aturan praktis untuk menambahkan / tidak menambahkan logika baru ke suatu fungsi? "Apakah menonaktifkannya akan menyebabkan efek samping bisnis utama?"
Andre Borges
2
If the purpose of my event would be to send new user data to Google Analytics - then disabling it would have the same business effect as disabling logging: not critical, but pretty upsetting . Bagaimana jika Anda menembakkan peristiwa prematur yang menyebabkan "berita" palsu. Bagaimana jika analitik memperhitungkan "pengguna" yang akhirnya tidak dibuat karena kesalahan dengan transaksi DB? Bagaimana jika perusahaan membuat keputusan atas lokasi yang salah, didukung oleh data yang tidak tepat? Anda terlalu fokus pada sisi teknis masalah ini. "Kadang-kadang Anda tidak dapat melihat kayu untuk pohon-pohon '"
Laiv
@ Longv, Anda membuat poin yang valid, tetapi ini bukan poin dari pertanyaan saya, atau jawaban ini. Pertanyaannya adalah apakah ini adalah solusi yang valid dalam konteks SRP, jadi mari kita asumsikan tidak ada kesalahan transaksi DB.
Andre Borges
Anda pada dasarnya meminta saya untuk memberi tahu Anda apa yang ingin Anda dengar. Saya hanya memberi Anda ruang lingkup. Ruang lingkup yang lebih luas bagi Anda untuk memutuskan apakah SRP penting atau tidak karena SRP tidak berguna tanpa konteks yang tepat. IMO cara Anda mendekati masalah tidak benar karena Anda hanya berfokus pada solusi teknis. Anda harus memberikan relevansi yang cukup untuk seluruh konteks. Dan ya, DB mungkin gagal. Ada kemungkinan hal itu terjadi dan Anda tidak boleh mengabaikannya, karena seperti yang Anda tahu, hal-hal terjadi dan hal-hal ini dapat mengubah pikiran Anda mengenai keraguan tentang SRP atau praktik baik lainnya.
Laiv
1
Yang mengatakan, ingatlah bahwa prinsip-prinsip bukanlah aturan yang ditulis dalam batu. Mereka permeabel (adaptif). Seperti yang Anda lihat, mereka terbuka untuk interpretasi. Peninjau Anda memiliki interpretasi dan Anda memiliki yang lain. Cobalah untuk melihat apa yang Anda lihat, selesaikan keraguan dan kekhawatirannya, atau biarkan dia menyelesaikannya. Anda tidak akan menemukan jawaban "benar" di sini. Jawaban yang benar terserah Anda dan pengulas Anda untuk menemukan, meminta terlebih dahulu persyaratan (fungsional dan non-fungsional) proyek.
Laiv
4

Secara teoritis, SRP adalah tentang orang , seperti yang dijelaskan Paman Bob dalam artikelnya The Single Responsibility Principle . Terima kasih Robert Harvey karena memberikannya dalam komentar Anda.

Pertanyaan yang benar adalah:

"Pemangku kepentingan" mana yang menambahkan persyaratan "kirim email"?

Jika pemangku kepentingan itu juga bertanggung jawab atas ketekunan data (tidak mungkin tetapi mungkin) maka ini tidak melanggar SRP. Kalau tidak, itu akan terjadi.

pengguna949300
sumber
2
Menarik - Saya tidak pernah mendengar penafsiran SRP ini. Apakah Anda memiliki petunjuk untuk informasi lebih lanjut / literatur tentang interpretasi ini?
sleske
2
@sleske: Dari Paman Bob sendiri : "Dan ini sampai pada inti dari Prinsip Tanggung Jawab Tunggal. Prinsip ini adalah tentang orang. Ketika Anda menulis modul perangkat lunak, Anda ingin memastikan bahwa ketika perubahan diminta, perubahan itu hanya dapat berasal dari satu orang, atau lebih tepatnya, satu kelompok orang yang sangat erat yang mewakili fungsi bisnis tunggal yang didefinisikan secara sempit. "
Robert Harvey
Terima kasih Robert. IMO, Nama "Prinsip Tanggung Jawab Tunggal" itu mengerikan, karena kedengarannya sederhana, tetapi terlalu sedikit orang menindaklanjuti apa yang dimaksud makna "tanggung jawab". Agak seperti bagaimana OOP telah bermutasi dari banyak konsep aslinya, dan sekarang istilah yang cukup berarti.
user949300
1
Ya. Itulah yang terjadi pada istilah REST. Bahkan Roy Fielding mengatakan orang menggunakannya salah.
Robert Harvey
Meskipun mengutip terkait, saya pikir jawaban ini tidak mengetahui bahwa persyaratan "mengirim email" bukanlah persyaratan langsung yang menjadi dasar pertanyaan pelanggaran SRP. Namun, dengan mengatakan "Siapa" pemangku kepentingan "yang menambahkan persyaratan" ajukan acara " , jawaban ini akan menjadi lebih terkait dengan pertanyaan aktual. Saya memodifikasi jawaban saya sendiri sedikit untuk membuatnya lebih jelas.
Doc Brown
2

Meskipun secara teknis tidak ada yang salah dengan repositori yang memberitahukan peristiwa, saya akan menyarankan untuk melihatnya dari sudut pandang fungsional di mana kenyamanannya menimbulkan kekhawatiran.

Membuat pengguna, memutuskan apa itu pengguna baru dan kegigihannya adalah 3 hal berbeda .

Tempat saya

Pertimbangkan premis sebelumnya sebelum memutuskan apakah repositori adalah tempat yang tepat untuk memberi tahu acara bisnis (terlepas dari SRP). Perhatikan bahwa saya mengatakan acara bisnis karena bagi saya UserCreatedmemiliki konotasi yang berbeda dari UserStoredatau UserAdded 1 . Saya juga akan menganggap masing-masing acara tersebut ditujukan kepada audiens yang berbeda.

Di satu sisi, membuat pengguna adalah aturan khusus bisnis yang mungkin melibatkan ketekunan. Ini mungkin melibatkan lebih banyak operasi bisnis, melibatkan lebih banyak operasi database / jaringan. Operasi lapisan ketekunan tidak menyadari. Lapisan persistensi tidak memiliki konteks yang cukup untuk memutuskan apakah use case berakhir dengan sukses atau tidak.

Di sisi lain, itu tidak selalu benar bahwa _dataContext.SaveChanges();telah berhasil mempertahankan pengguna. Ini akan tergantung pada rentang transaksi basis data. Misalnya, bisa jadi benar untuk database seperti MongoDB, yang transaksi bersifat atomik, tetapi tidak bisa, untuk RDBMS tradisional yang menerapkan transaksi ACID di mana mungkin ada lebih banyak transaksi yang terlibat dan belum berkomitmen.

Apakah ini poin yang valid?

Bisa jadi. Namun, saya berani mengatakan bahwa ini bukan hanya masalah SRP (secara teknis), tetapi juga masalah kenyamanan (berbicara secara fungsional).

  • Apakah nyaman untuk memecat acara bisnis dari komponen yang tidak mengetahui operasi bisnis yang sedang berlangsung?
  • Apakah mereka mewakili tempat yang tepat sebanyak saat yang tepat untuk melakukannya?
  • Haruskah saya membiarkan komponen-komponen ini mengatur logika bisnis saya melalui pemberitahuan seperti ini?
  • Bisakah saya membatalkan efek samping yang disebabkan oleh kejadian prematur? 2

Bagi saya, membangkitkan acara di sini pada dasarnya sama dengan logging

Benar-benar tidak. Namun pembalakan tidak memiliki efek samping, karena Anda menyarankan acara UserCreatedtersebut kemungkinan akan menyebabkan operasi bisnis lainnya terjadi. Suka notifikasi. 3

ia hanya mengatakan bahwa logika seperti itu harus dienkapsulasi di kelas lain, dan tidak masalah jika repositori memanggil kelas-kelas lain ini

Belum tentu benar. SRP bukan hanya masalah kelas khusus. Ini beroperasi pada berbagai tingkat abstraksi, seperti lapisan, perpustakaan dan sistem! Ini tentang kohesi, tentang menjaga bersama apa yang berubah karena alasan yang sama oleh para pemangku kepentingan yang sama . Jika kreasi pengguna ( use case ) berubah, itu kemungkinan momen dan alasan terjadinya peristiwa juga berubah.


1: Penamaan hal-hal yang memadai juga penting.

2: Katakanlah kami mengirim UserCreatedsetelah _dataContext.SaveChanges();, tetapi seluruh transaksi basis data gagal kemudian karena masalah koneksi atau pelanggaran kendala. Berhati-hatilah dengan penyiaran acara yang prematur, karena efek sampingnya bisa sulit untuk dibatalkan (jika itu mungkin).

3: Proses pemberitahuan yang tidak ditangani dengan baik dapat menyebabkan Anda memecat notifikasi yang tidak dapat dibatalkan / didukung>

Laiv
sumber
1
+1 Poin yang sangat bagus tentang rentang transaksi. Mungkin terlalu dini untuk menegaskan bahwa pengguna telah dibuat, karena kembalikan dapat terjadi; dan tidak seperti log, kemungkinan sebagian aplikasi melakukan sesuatu dengan acara tersebut.
Andres F.
2
Persis. Acara menunjukkan kepastian. Sesuatu terjadi tetapi ini sudah berakhir.
Laiv
1
@Laiv: Kecuali kalau tidak. Microsoft memiliki semua jenis acara yang diawali Beforeatau Previewyang tidak memberikan jaminan sama sekali tentang kepastian.
Robert Harvey
1
@ jpmc26: Tanpa alternatif, saran Anda tidak membantu.
Robert Harvey
1
@ jpmc26: Jadi jawaban Anda adalah "berubah menjadi ekosistem pembangunan yang sama sekali berbeda dengan seperangkat alat dan karakteristik kinerja yang sama sekali berbeda." Panggil saya sebaliknya, tetapi saya akan membayangkan ini tidak mungkin untuk sebagian besar upaya pembangunan.
Robert Harvey
1

Tidak, ini tidak melanggar SRP.

Banyak yang berpikir bahwa Prinsip Tanggung Jawab Tunggal berarti suatu fungsi hanya boleh melakukan "satu hal", dan kemudian terjebak dalam diskusi tentang apa yang merupakan "satu hal".

Tetapi bukan itu yang dimaksud prinsip. Ini tentang masalah tingkat bisnis. Kelas tidak boleh menerapkan beberapa masalah atau persyaratan yang dapat berubah secara independen di tingkat bisnis. Katakanlah suatu kelas baik menyimpan pengguna dan mengirim pesan selamat datang hardcoded melalui email. Berbagai masalah independen dapat menyebabkan persyaratan kelas seperti itu berubah. Perancang dapat meminta html / stylesheet surat untuk diubah. Pakar komunikasi dapat meminta susunan kata dari surat untuk diubah. Dan pakar UX dapat memutuskan bahwa surat seharusnya dikirim pada titik yang berbeda dalam aliran orientasi. Jadi kelas dapat berubah beberapa persyaratan dari sumber independen. Ini melanggar SRP.

Namun memecat suatu acara tidak melanggar SRP, karena acara tersebut hanya bergantung pada penyelamatan pengguna dan bukan pada masalah lainnya. Acara sebenarnya adalah cara yang sangat baik untuk menegakkan SRP, karena Anda dapat memiliki email yang dipicu oleh save tanpa repositori dipengaruhi oleh - atau bahkan mengetahui tentang - surat.

JacquesB
sumber
1

Jangan khawatir tentang prinsip tanggung jawab tunggal. Ini tidak akan membantu Anda membuat keputusan yang baik di sini karena Anda dapat secara subyektif memilih konsep tertentu sebagai "tanggung jawab." Anda bisa mengatakan tanggung jawab kelas adalah mengelola kegigihan data ke database, atau Anda bisa mengatakan tanggung jawabnya adalah untuk melakukan semua pekerjaan yang terkait dengan menciptakan pengguna. Ini hanya level yang berbeda dari perilaku aplikasi, dan keduanya merupakan ekspresi konseptual yang valid dari "tanggung jawab tunggal." Jadi prinsip ini tidak membantu untuk menyelesaikan masalah Anda.

Prinsip yang paling berguna untuk diterapkan dalam kasus ini adalah prinsip yang paling tidak mengejutkan . Jadi mari kita ajukan pertanyaan: apakah mengherankan bahwa repositori dengan peran utama mempertahankan data ke database juga mengirim email?

Ya, itu sangat mengejutkan. Ini adalah dua sistem eksternal yang sepenuhnya terpisah, dan namanya SaveChangestidak menyiratkan juga mengirim pemberitahuan. Fakta bahwa Anda mendelegasikan hal ini ke suatu acara membuat perilaku tersebut bahkan lebih mengejutkan, karena seseorang yang membaca kode tidak lagi dapat dengan mudah melihat perilaku tambahan apa yang muncul. Ketidaksadaran merusak keterbacaan. Terkadang, manfaatnya sebanding dengan biaya keterbacaan, tetapi tidak ketika Anda secara otomatis menggunakan sistem eksternal tambahan yang memiliki efek yang dapat diamati oleh pengguna akhir. (Logging dapat dikecualikan di sini karena efeknya pada dasarnya adalah pencatatan untuk tujuan debugging. Pengguna akhir tidak mengkonsumsi log, sehingga tidak ada salahnya selalu login.) Lebih buruk lagi, ini mengurangi fleksibilitas dalam waktu mengirim email, sehingga tidak memungkinkan untuk melakukan interleave operasi lain antara save dan notifikasi.

Jika kode Anda biasanya perlu mengirim pemberitahuan ketika pengguna berhasil dibuat, Anda dapat membuat metode yang melakukannya:

public void AddUserAndNotify(IUserRepository repo, IEmailNotification notifier, MyUser user)
{
    repo.Add(user);
    repo.SaveChanges();
    notifier.SendUserCreatedNotification(user);
}

Tetapi apakah ini menambah nilai tergantung pada spesifikasi aplikasi Anda.


Saya sebenarnya tidak menyarankan keberadaan SaveChangesmetode ini sama sekali. Metode ini mungkin akan melakukan transaksi database, tetapi repositori lain mungkin telah memodifikasi database dalam transaksi yang sama . Fakta bahwa semua itu dilakukan, sekali lagi mengejutkan, karena SaveChangessecara spesifik terkait dengan repositori pengguna ini.

Pola paling mudah untuk mengelola transaksi basis data adalah usingblok luar :

using (DataContext context = new DataContext())
{
    _userRepository.Add(context, user);
    context.SaveChanges();
    notifier.SendUserCreatedNotification(user);
}

Ini memberi programmer kontrol eksplisit atas ketika perubahan untuk semua repositori disimpan, memaksa kode untuk secara eksplisit mendokumentasikan urutan peristiwa yang harus terjadi sebelum komit, memastikan rollback dikeluarkan karena kesalahan (dengan asumsi bahwa DataContext.Disposemengeluarkan rollback), dan menghindari disembunyikan koneksi antara kelas stateful.

Saya juga lebih suka tidak mengirim email langsung dalam permintaan. Akan lebih kuat untuk mencatat perlunya pemberitahuan dalam antrian. Ini akan memungkinkan penanganan kegagalan yang lebih baik. Secara khusus, jika terjadi kesalahan dalam mengirim email, itu dapat dicoba lagi nanti tanpa mengganggu menyelamatkan pengguna, dan menghindari kasus di mana pengguna dibuat tetapi kesalahan dikembalikan oleh situs.

using (DataContext context = new DataContext())
{
    _userRepository.Add(context, user);
    _emailNotificationQueue.AddUserCreateNotification(user);
    _emailNotificationQueue.Commit();
    context.SaveChanges();
}

Lebih baik untuk melakukan antrian pemberitahuan terlebih dahulu karena konsumen antrian dapat memverifikasi bahwa pengguna ada sebelum mengirim e-mail, jika context.SaveChanges()panggilan gagal. (Kalau tidak, Anda akan memerlukan strategi komitmen dua fase penuh untuk menghindari Heisenbugs.)


Intinya adalah bersikap praktis. Sebenarnya pikirkan konsekuensi (baik dari segi risiko dan manfaat) dari menulis kode dengan cara tertentu. Saya menemukan bahwa "prinsip tanggung jawab tunggal" tidak sering membantu saya melakukan itu, sementara "prinsip kejutan paling tidak" sering membantu saya masuk ke kepala pengembang lain (untuk berbicara) dan berpikir tentang apa yang mungkin terjadi.

jpmc26
sumber
4
Apakah mengejutkan bahwa repositori dengan peran utama mempertahankan data ke database juga mengirim e-mail - Saya pikir Anda melewatkan inti pertanyaan saya. Gudang saya tidak mengirim email. Itu hanya memunculkan suatu peristiwa, dan bagaimana acara ini ditangani - adalah tanggung jawab kode eksternal.
Andre Borges
4
Pada dasarnya Anda membuat argumen "jangan gunakan acara."
Robert Harvey
3
[mengangkat bahu] Acara adalah pusat dari sebagian besar kerangka kerja UI. Hilangkan acara, dan kerangka kerja itu tidak berfungsi sama sekali.
Robert Harvey
2
@ jpmc26: Ini disebut ASP.NET Webforms. Menyebalkan sekali.
Robert Harvey
2
My repository is not sending emails. It just raises an eventsebab-akibat. Repositori memicu proses notifikasi.
Laiv
0

Saat ini SaveChangesmelakukan dua hal: menyimpan perubahan dan mencatatnya. Sekarang Anda ingin menambahkan hal lain: kirim pemberitahuan email.

Anda memiliki ide cerdas untuk menambahkan acara, tetapi ini dikritik karena melanggar Prinsip Tanggung Jawab Tunggal (SRP), tanpa memperhatikan bahwa itu sudah dilanggar.

Untuk mendapatkan solusi SRP murni, pertama-tama picu acara tersebut, lalu panggil semua kait untuk acara itu, yang sekarang ada tiga: menyimpan, masuk, dan akhirnya mengirim email.

Entah Anda yang memicu acara terlebih dahulu, atau Anda harus menambahkannya SaveChanges. Solusi Anda adalah campuran antara keduanya. Itu tidak mengatasi pelanggaran yang ada sementara itu mendorong mencegahnya meningkat melampaui tiga hal. Refactoring kode yang ada untuk mematuhi SRP mungkin memerlukan lebih banyak pekerjaan daripada yang diperlukan. Terserah proyek Anda seberapa jauh mereka ingin mengambil SRP.

CJ Dennis
sumber
-1

Kode sudah melanggar SRP - kelas yang sama bertanggung jawab untuk berkomunikasi dengan konteks data dan pencatatan.

Anda hanya memutakhirkannya menjadi 3 tanggung jawab.

Salah satu cara untuk melucuti sesuatu menjadi tanggung jawab adalah dengan mengabstraksikannya _userRepository; menjadikannya penyiar perintah.

Ini memiliki seperangkat perintah, ditambah satu set pendengar. Ia mendapat perintah, dan menyiarkannya ke pendengarnya. Mungkin para pendengar itu diperintahkan, dan mungkin mereka bahkan bisa mengatakan perintah gagal (yang pada gilirannya disiarkan kepada pendengar yang sudah diberitahu).

Sekarang, sebagian besar perintah mungkin hanya memiliki 1 pendengar (konteks data). SaveChanges, sebelum perubahan Anda, memiliki 2 - konteks data, dan kemudian logger.

Perubahan Anda lalu menambahkan pendengar lain untuk menyimpan perubahan, yaitu meningkatkan acara yang dibuat pengguna baru di layanan acara.

Ada beberapa manfaatnya. Sekarang Anda dapat menghapus, meningkatkan, atau mereplikasi kode logging tanpa sisa kode Anda peduli. Anda dapat menambahkan lebih banyak pemicu di simpan perubahan untuk lebih banyak hal yang membutuhkannya.

Semua ini diputuskan ketika _userRepositorydibuat dan ditransfer (atau, mungkin, fitur-fitur tambahan itu ditambahkan / dihapus dengan cepat; dapat menambah / meningkatkan logging sementara aplikasi berjalan dapat digunakan).

Yakk
sumber