Kapan paradigma “Do One Thing” menjadi berbahaya?

21

Demi argumen, inilah fungsi sampel yang mencetak konten file yang diberikan baris demi baris.

Versi 1:

void printFile(const string & filePath) {
  fstream file(filePath, ios::in);
  string line;
  while (std::getline(file, line)) {
    cout << line << endl;
  }
}

Saya tahu direkomendasikan bahwa fungsi melakukan satu hal pada satu tingkat abstraksi. Bagi saya, meskipun kode di atas tidak cukup banyak satu hal dan cukup atom.

Beberapa buku (seperti Kode Bersih Robert C. Martin) tampaknya menyarankan memecah kode di atas menjadi fungsi yang terpisah.

Versi 2:

void printFile(const string & filePath) {
  fstream file(filePath, ios::in);
  printLines(file);
}

void printLines(fstream & file) {
  string line;
  while (std::getline(file, line)) {
    printLine(line);
  }
}

void printLine(const string & line) {
  cout << line << endl;
}

Saya mengerti apa yang ingin mereka capai (buka file / baca baris / print line), tetapi bukankah itu sedikit berlebihan?

Versi aslinya sederhana dan dalam beberapa hal sudah melakukan satu hal - mencetak file.

Versi kedua akan menghasilkan sejumlah besar fungsi yang sangat kecil yang mungkin jauh lebih tidak terbaca dari versi pertama.

Bukankah, dalam hal ini, lebih baik memiliki kode di satu tempat?

Pada titik mana paradigma "Do One Thing" menjadi berbahaya?

Petr
sumber
13
Praktik pengkodean semacam ini selalu didasarkan pada kasus per kasus. Tidak pernah ada pendekatan tunggal.
iammilind
1
@Alex - Jawaban yang diterima secara harfiah tidak ada hubungannya dengan pertanyaan. Saya menemukan itu sangat aneh.
ChaosPandion
2
Saya perhatikan bahwa versi refactored Anda terbalik, yang berkontribusi pada kurangnya keterbacaan. Membaca bawah file, Anda akan mengharapkan untuk melihat printFile, printLinesdan akhirnya printLine.
Anthony Pegram
1
@ Kev, saya sekali lagi hanya bisa tidak setuju, terutama dengan kategorisasi itu. Ini bukan kesedihan, itu intinya! OP-lah yang secara spesifik mengatakan versi kedua mungkin tidak bisa dibaca. OP yang secara khusus mengutip Clean Code sebagai inspirasi untuk versi kedua. Komentar saya pada dasarnya adalah bahwa Kode Bersih tidak akan membuatnya menulis kode seperti itu. Urutan sebenarnya penting untuk keterbacaan, Anda membaca file seperti Anda membaca artikel surat kabar, semakin detail sampai Anda pada dasarnya tidak tertarik.
Anthony Pegram
1
Seperti Anda tidak akan berharap untuk membaca puisi mundur, Anda juga tidak akan berharap untuk melihat tingkat detail terendah sebagai hal pertama di dalam kelas tertentu. Untuk maksud Anda, kode ini membutuhkan sedikit waktu untuk memilah dengan cepat, tetapi saya hanya akan menganggap kode ini bukan satu-satunya kode yang akan ia tulis. Menurut saya, jika dia akan mengutip Clean Code, paling tidak yang bisa dia lakukan adalah mengikutinya. Jika kodenya rusak, tentu itu akan lebih mudah dibaca daripada sebaliknya.
Anthony Pegram

Jawaban:

15

Tentu saja, ini hanya menimbulkan pertanyaan "Apa itu satu hal?" Apakah membaca satu baris adalah satu hal dan menulis satu baris lagi? Atau menyalin garis dari satu aliran ke yang lain dianggap satu hal? Atau menyalin file?

Tidak ada jawaban yang sulit dan obyektif untuk itu. Terserah kamu. Anda bisa memutuskan. Anda harus memutuskan. Tujuan utama paradigma "do one thing" mungkin untuk menghasilkan kode yang semudah mungkin dipahami, sehingga Anda dapat menggunakannya sebagai pedoman. Sayangnya, ini juga tidak dapat diukur secara objektif, jadi harus mengandalkan firasat Anda dan "WTF?" hitung dalam ulasan kode .

IMO suatu fungsi yang terdiri dari hanya satu baris kode jarang yang sepadan dengan masalahnya. Anda printLine()tidak memiliki keuntungan menggunakan std::cout << line << '\n'1 secara langsung. Jika saya melihat printLine(), saya harus menganggapnya melakukan apa yang dikatakan namanya, atau mencarinya dan memeriksa. Jika saya melihat std::cout << line << '\n', saya langsung tahu apa fungsinya, karena ini adalah cara kanonik untuk mengeluarkan konten string sebagai baris std::cout.

Namun, tujuan penting lain dari paradigma adalah untuk memungkinkan penggunaan kembali kode, dan itu ukuran yang jauh lebih objektif. Misalnya, dalam versi ke-2 Anda printLines() dapat dengan mudah ditulis sehingga merupakan algoritme yang berguna secara universal yang menyalin garis dari satu aliran ke yang lain:

void copyLines(std::istream& is, std::ostream& os)
{
  std::string line;
  while( std::getline(is, line) );
    os << line << '\n';
  }
}

Algoritma seperti itu dapat digunakan kembali dalam konteks lain juga.

Anda kemudian dapat meletakkan segala sesuatu yang spesifik untuk kasing yang satu ini ke dalam fungsi yang memanggil algoritma umum ini:

void printFile(const std::string& filePath) {
  std::ifstream file(filePath.c_str());
  printLines(file, std::cout);
}

1 Perhatikan bahwa saya menggunakan '\n'daripada std::endl. '\n'seharusnya menjadi pilihan default untuk menghasilkan baris baru , std::endladalah kasus aneh .

sbi
sumber
2
+1 - Saya sebagian besar setuju, tapi saya pikir ada lebih dari sekadar "firasat". Masalahnya adalah ketika orang menilai "satu hal" dengan menghitung detail implementasi. Bagi saya, fungsi harus mengimplementasikan (dan namanya menggambarkan) abstraksi tunggal yang jelas. Anda seharusnya tidak pernah memberi nama fungsi "do_x_and_y". Implementasinya dapat dan harus melakukan beberapa hal (lebih sederhana) - dan masing-masing hal yang lebih sederhana dapat diuraikan menjadi beberapa hal yang lebih sederhana dan seterusnya. Itu hanya dekomposisi fungsional dengan aturan tambahan - bahwa fungsi (dan namanya) masing-masing harus menggambarkan konsep / tugas / tunggal yang jelas.
Steve314
@ Steve314: Saya belum mendaftarkan detail implementasi sebagai kemungkinan. Menyalin garis dari satu aliran ke aliran lainnya dengan jelas adalah abstraksi satu hal . Atau itu? Dan mudah untuk menghindarinya do_x_and_y()dengan menyebutkan fungsinya do_everything(). Ya, itu adalah contoh konyol, tetapi itu menunjukkan bahwa aturan ini gagal bahkan mencegah contoh paling buruk dari desain buruk. IMO ini adalah keputusan perasaan usus sebanyak yang ditentukan oleh konvensi. Kalau tidak, jika itu obyektif, Anda bisa menghasilkan metrik untuk itu - yang Anda tidak bisa.
sbi
1
Saya tidak bermaksud untuk kontradiksi - hanya untuk menyarankan tambahan. Saya kira apa yang saya lupa katakan adalah bahwa, dari pertanyaan, penguraian ke printLinedll adalah valid - masing-masing adalah abstraksi tunggal - tetapi itu tidak berarti perlu. printFilesudah "satu hal". Meskipun Anda dapat menguraikannya menjadi tiga abstraksi level bawah yang terpisah, Anda tidak perlu mendekomposisi pada setiap level abstraksi yang memungkinkan . Setiap fungsi harus melakukan "satu hal", tetapi tidak setiap kemungkinan "satu hal" perlu menjadi fungsi. Memindahkan terlalu banyak kerumitan ke grafik panggilan itu sendiri bisa menjadi masalah.
Steve314
7

Memiliki fungsi hanya melakukan "satu hal" adalah sarana untuk dua tujuan yang diinginkan, bukan perintah dari Tuhan:

  1. Jika fungsi Anda hanya melakukan "satu hal", itu akan membantu Anda menghindari duplikasi kode dan mengasapi API karena Anda akan dapat menyusun fungsi untuk menyelesaikan hal-hal yang lebih rumit daripada melakukan ledakan kombinatorial pada level yang lebih tinggi, fungsi yang lebih sedikit komposisinya .

  2. Memiliki fungsi hanya melakukan "satu hal" dapat membuat kode lebih mudah dibaca. Ini tergantung pada apakah Anda mendapatkan lebih banyak kejelasan dan kemudahan bernalar dengan memisahkan hal-hal daripada Anda kehilangan verbosity, tipuan dan overhead konseptual dari konstruksi yang memungkinkan Anda untuk memisahkan hal-hal.

Karena itu, "satu hal" tidak dapat dihindari subjektif dan tergantung pada tingkat abstraksi apa yang relevan dengan program Anda. Jika printLinesdianggap sebagai operasi tunggal yang mendasar dan satu-satunya cara mencetak garis yang Anda pedulikan atau perhatikan, maka untuk tujuan Anda printLineshanya melakukan satu hal. Kecuali jika Anda menemukan versi kedua lebih mudah dibaca (saya tidak) versi pertama baik-baik saja.

Jika Anda mulai membutuhkan kontrol lebih besar atas level abstraksi yang lebih rendah dan berakhir dengan duplikasi halus dan ledakan kombinatorial (yaitu a printLinesuntuk nama file dan yang benar-benar terpisah printLinesuntuk fstreamobjek, printLinesuntuk konsol dan printLinesuntuk file) maka printLinesmelakukan lebih dari satu hal di level abstraksi yang Anda sayangi.

dsimcha
sumber
Saya akan menambahkan yang ketiga dan fungsi yang lebih kecil lebih mudah diuji. Karena mungkin ada lebih sedikit input yang diperlukan jika fungsi hanya melakukan satu hal, membuatnya lebih mudah untuk mengujinya secara mandiri.
PersonalNexus
@PersonalNexus: Saya agak setuju pada masalah pengujian, tapi IMHO itu konyol untuk menguji detail implementasi. Bagi saya tes unit harus menguji "satu hal" sebagaimana didefinisikan dalam jawaban saya. Apa pun yang berbutir halus membuat tes Anda rapuh (karena mengubah detail implementasi akan memerlukan tes Anda untuk berubah) dan kode Anda secara verbal kasar, tidak langsung, dll. (Karena Anda akan menambahkan tipuan hanya untuk mendukung pengujian).
dsimcha
6

Pada skala ini, itu tidak masalah. Implementasi fungsi tunggal sangat jelas dan dapat dimengerti. Namun, menambahkan sedikit lebih banyak kerumitan membuatnya sangat menarik untuk memisahkan iterasi dari tindakan. Misalnya, Anda perlu mencetak baris dari serangkaian file yang ditentukan oleh pola seperti "* .txt". Lalu saya akan memisahkan iterasi dari tindakan:

printLines(FileSet files) {
   files.each({ 
       file -> file.eachLine({ 
           line -> printLine(line); 
       })
   })
}

Sekarang file iterasi dapat diuji secara terpisah.

Saya membagi fungsi untuk menyederhanakan pengujian, atau untuk meningkatkan keterbacaan. Jika tindakan yang dilakukan pada setiap baris data cukup kompleks untuk menjamin komentar, maka saya pasti akan membaginya menjadi fungsi yang terpisah.

kevin cline
sumber
4
Saya pikir Anda berhasil. Jika kita membutuhkan komentar untuk menjelaskan sebuah baris, maka selalu saatnya untuk mengekstrak suatu metode.
Roger CS Wernersson
5

Ekstrak metode ketika Anda merasa perlu komentar untuk menjelaskan sesuatu.

Tulis metode yang hanya melakukan apa yang namanya mereka katakan dengan cara yang jelas, atau menceritakan sebuah kisah dengan memanggil metode yang disebut dengan cerdik.

Roger CS Wernersson
sumber
3

Bahkan dalam kasus sederhana Anda, Anda kehilangan detail bahwa Prinsip Tanggung Jawab Tunggal akan membantu Anda mengelola dengan lebih baik. Misalnya, apa yang terjadi ketika ada masalah dengan membuka file. Menambahkan penanganan pengecualian untuk mengeras terhadap kasus tepi akses file akan menambah 7-10 baris kode ke fungsi Anda.

Setelah Anda membuka file, Anda masih tidak aman. Itu bisa ditarik dari Anda (terutama jika itu file di jaringan), Anda bisa kehabisan memori, lagi sejumlah kasus tepi dapat terjadi yang ingin Anda keraskan dan akan menggembungkan fungsi monolitik Anda.

Satu-liner, printline tampaknya cukup berbahaya. Tetapi ketika fungsionalitas baru ditambahkan ke printer file (parsing dan memformat teks, render ke berbagai jenis tampilan, dll.) Itu akan tumbuh dan Anda akan berterima kasih kepada diri sendiri nanti.

Tujuan SRP adalah memungkinkan Anda untuk memikirkan satu tugas pada satu waktu. Ini seperti memecah blok teks yang besar menjadi beberapa paragraf sehingga pembaca dapat memahami poin yang ingin Anda sampaikan. Dibutuhkan sedikit lebih banyak waktu untuk menulis kode yang menganut prinsip-prinsip ini. Tetapi dengan melakukan itu kami membuatnya lebih mudah untuk membaca kode itu. Pikirkan betapa bahagianya diri Anda di masa depan ketika ia harus melacak bug dalam kode dan menemukannya dipartisi dengan rapi.

Michael Brown
sumber
2
Saya memilih jawaban ini karena saya suka logikanya walaupun saya tidak setuju dengan itu! Berikan struktur berdasarkan pada pemikiran kompleks tentang apa yang mungkin terjadi di masa depan adalah kontra produktif. Buat kode faktor bila perlu. Jangan abstraksi sampai Anda perlu melakukannya. Kode modern diganggu oleh orang-orang yang berusaha dengan patuh mengikuti aturan alih-alih hanya menulis kode yang berfungsi dan mengadaptasinya dengan enggan . Pemrogram yang baik malas .
Yttrill
Terima kasih atas komentarnya. Catatan Saya tidak menganjurkan abstraksi dini, hanya membagi operasi logis sehingga lebih mudah untuk melakukannya nanti.
Michael Brown
2

Saya pribadi lebih suka pendekatan yang terakhir, karena itu menyelamatkan Anda bekerja di masa depan dan memaksa "bagaimana melakukannya dengan cara yang umum" pola pikir. Meskipun demikian, dalam kasus Anda, Versi 1 lebih baik daripada Versi 2 - hanya karena masalah yang dipecahkan oleh Versi 2 terlalu sepele dan spesifik untuk arus. Saya pikir itu harus dilakukan dengan cara berikut (termasuk perbaikan bug yang diusulkan oleh Nawaz):

Fungsi utilitas umum:

void printLine(ostream& output, const string & line) { 
    output << line << endl; 
} 

void printLines(istream& input, ostream& output) { 
    string line; 
    while (getline(input, line)) {
        printLine(output, line); 
    } 
} 

Fungsi khusus domain:

void printFile(const string & filePath, ostream& output = std::cout) { 
    fstream file(filePath, ios::in); 
    printLines(file, output); 
} 

Sekarang printLinesdan printLinedapat bekerja tidak hanya dengan fstream, tetapi dengan aliran apa pun.

gwiazdorrr
sumber
2
Saya tidak setuju. Bahwa printLine()fungsi tidak memiliki nilai. Lihat jawaban saya .
sbi
1
Nah, jika kita menyimpan printLine () maka kita dapat menambahkan dekorator yang menambahkan nomor baris atau pewarnaan sintaks. Karena itu, saya tidak akan mengekstraksi metode itu sampai saya menemukan alasannya.
Roger CS Wernersson
2

Setiap paradigma , (bukan hanya yang Anda kutip) yang harus diikuti memerlukan disiplin, dan dengan demikian - mengurangi "kebebasan berbicara" - menghasilkan overhead awal (setidaknya hanya karena Anda harus mempelajarinya!). Dalam pengertian ini setiap paradigma dapat menjadi berbahaya ketika biaya overhead itu tidak dikompensasikan oleh keuntungan paradigma dirancang untuk tetap dengan dirinya sendiri.

Dengan demikian, jawaban yang benar untuk pertanyaan itu membutuhkan kemampuan yang baik untuk "meramalkan" masa depan, seperti:

  • Saya sekarang harus melakukan AdanB
  • Berapa probabilitasnya, dalam waktu dekat saya akan diminta untuk melakukan juga A-dan B+(yaitu sesuatu yang terlihat seperti A dan B, tetapi hanya sedikit berbeda)?
  • Berapa probabilitas di masa depan yang lebih jauh, yang akan menjadi A + A* atau A*-?

Jika probabilitas itu relatif tinggi, itu akan menjadi peluang yang baik jika - sambil memikirkan A dan B - saya juga memikirkan kemungkinan varian mereka, dengan demikian untuk mengisolasi bagian-bagian umum sehingga saya akan dapat menggunakannya kembali.

Jika probabilitas itu sangat rendah (varian apa pun di sekitar Apada dasarnya tidak lebih dariA dirinya sendiri), pelajari cara menguraikan A lebih jauh kemungkinan besar akan menghasilkan waktu yang terbuang.

Sebagai contoh, izinkan saya menceritakan kisah nyata ini kepada Anda:

Selama kehidupan masa lalu saya sebagai seorang guru, saya menemukan bahwa - pada sebagian besar proyek siswa - praktis semuanya menyediakan fungsi mereka sendiri untuk menghitung panjang string C .

Setelah beberapa penyelidikan saya menemukan bahwa, sebagai masalah yang sering terjadi, semua siswa datang ke ide untuk menggunakan fungsi untuk itu. Setelah memberi tahu mereka bahwa ada fungsi perpustakaan untuk itu (strlen ), banyak dari mereka menjawab bahwa karena masalahnya sangat sederhana dan sepele, lebih efektif bagi mereka untuk menulis fungsi mereka sendiri (2 baris kode) daripada mencari buku pedoman C library. (Itu tahun 1984, lupakan WEB dan google!) dengan urutan alfabet yang ketat untuk melihat apakah ada fungsi yang siap untuk itu.

Ini adalah contoh di mana paradigma "jangan menemukan kembali roda" dapat menjadi berbahaya, tanpa katalog roda yang efektif!

Emilio Garavaglia
sumber
2

Contoh Anda baik-baik saja untuk digunakan dalam alat pembuangan yang diperlukan kemarin untuk melakukan beberapa tugas tertentu. Atau sebagai alat administrasi yang dikendalikan langsung oleh administrator. Sekarang membuatnya kuat agar cocok untuk pelanggan Anda.

Tambahkan penanganan kesalahan / pengecualian yang tepat dengan pesan yang bermakna. Mungkin Anda memerlukan verifikasi parameter, termasuk keputusan yang harus dibuat, misalnya cara menangani file yang tidak ada. Tambahkan fungsionalitas logging, mungkin dengan level berbeda seperti info dan debugging. Tambahkan komentar sehingga kolega tim Anda tahu apa yang terjadi di sana. Tambahkan semua bagian yang biasanya dihilangkan untuk singkatnya dan dibiarkan sebagai latihan untuk pembaca ketika memberikan contoh kode. Jangan lupakan tes unit Anda.

Fungsi kecil Anda yang bagus dan cukup linier tiba-tiba berakhir dalam kekacauan kompleks yang memohon untuk dipisahkan menjadi fungsi terpisah.

Aman
sumber
2

IMO itu menjadi berbahaya ketika sejauh itu bahwa suatu fungsi hampir tidak melakukan apa-apa sama sekali kecuali mendelegasikan pekerjaan ke fungsi lain, karena itu pertanda bahwa itu bukan lagi abstraksi dari apa pun dan pola pikir yang mengarah ke fungsi-fungsi seperti itu selalu dalam bahaya melakukan hal-hal buruk ...

Dari pos asli

void printLine(const string & line) {
  cout << line << endl;
}

Jika Anda cukup bertele-tele, Anda mungkin memperhatikan bahwa printLine masih melakukan dua hal: menulis baris ke cout dan menambahkan karakter "garis akhir". Beberapa orang mungkin ingin mengatasinya dengan membuat fungsi baru:

void printLine(const string & line) {
  reallyPrintLine(line);
  addEndLine();
}

void reallyPrintLine(const string & line) {
  cout << line;
}

void addEndLine() {
  cout << endl;
}

Oh tidak, sekarang kami telah memperburuk masalahnya! Sekarang bahkan LUAR BIASA bahwa printLine melakukan DUA hal !!! 1! Tidaklah banyak kebodohan untuk menciptakan "cara kerja" yang paling tidak masuk akal yang bisa dibayangkan orang hanya untuk menyingkirkan masalah yang tak terhindarkan itu dengan mencetak sebuah garis yang terdiri dari mencetak garis itu sendiri dan menambahkan karakter end-of-line.

void printLine(const string & line) {
  for (int i=0; i<2; i++)
    reallyPrintLine(line, i);
}

void reallyPrintLine(const string & line, int action) {
  cout << (action==0?line:endl);
}
pengguna281377
sumber
1

Jawaban singkat ... itu tergantung.

Pikirkan tentang ini: bagaimana jika, di masa depan, Anda tidak ingin mencetak hanya ke output standar, tetapi ke file.

Saya tahu apa itu YAGNI, tapi saya hanya mengatakan mungkin ada kasus di mana beberapa implementasi diketahui diperlukan, tetapi ditunda. Jadi mungkin arsitek atau apa pun yang tahu fungsi itu harus dapat juga mencetak ke file, tetapi tidak ingin melakukan implementasi sekarang. Jadi dia menciptakan fungsi tambahan ini jadi, di masa depan, Anda hanya perlu mengubah output di satu tempat. Masuk akal?

Namun jika Anda yakin Anda hanya perlu keluaran di konsol, itu tidak masuk akal. Menulis "bungkus" cout <<sepertinya tidak berguna.

Luchian Grigore
sumber
1
Tapi sebenarnya, bukankah fungsi printLine tingkat abstraksi yang berbeda dari iterasi garis?
@Petr Saya kira begitu, itulah sebabnya mereka menyarankan Anda memisahkan fungsionalitasnya. Saya pikir konsepnya benar, tetapi Anda perlu menerapkannya berdasarkan kasus per kasus.
1

Seluruh alasan bahwa ada buku-buku yang menyediakan bab untuk kebajikan "melakukan satu hal" adalah bahwa masih ada pengembang di luar sana yang menulis fungsi yang panjangnya 4 halaman dan sarangnya memiliki 6 level. Jika kode Anda sederhana dan jelas, Anda telah melakukannya dengan benar.

Kevin
sumber
0

Seperti komentar poster lain, melakukan satu hal adalah masalah skala.

Saya juga menyarankan bahwa ide One Thing adalah menghentikan orang-orang yang mengkode efek samping. Ini dicontohkan oleh penggabungan berurutan di mana metode harus dipanggil dalam urutan tertentu untuk mendapatkan hasil yang 'benar'.

NWS
sumber