Dalam ulasan kode, haruskah peninjau selalu memberikan solusi untuk masalah? [Tutup]

Saat meninjau kode, saya biasanya mencoba membuat rekomendasi khusus tentang cara menyelesaikan masalah. Tetapi karena waktu yang terbatas yang dapat dihabiskan seseorang untuk meninjau, ini tidak selalu bekerja dengan baik. Dalam kasus ini saya merasa lebih efisien jika pengembang sendiri yang membuat solusi.

Hari ini saya meninjau beberapa kode dan menemukan bahwa kelas jelas tidak dirancang dengan baik. Itu memiliki sejumlah atribut opsional yang hanya ditugaskan untuk objek tertentu dan dibiarkan kosong untuk yang lain. Cara standar untuk menyelesaikan ini adalah dengan membagi kelas dan menggunakan warisan. Namun dalam kasus khusus ini solusi ini tampaknya terlalu rumit. Saya sendiri tidak terlibat dalam pengembangan perangkat lunak ini dan tidak terbiasa dengan semua modul. Karena itu saya tidak merasa cukup berpengetahuan untuk membuat keputusan tertentu.

Kasus khas lain yang saya alami berkali-kali adalah saya menemukan fungsi, kelas atau nama variabel yang jelas tidak berarti atau bahkan menyesatkan, tetapi saya sendiri tidak dapat menemukan nama yang bagus.

Jadi secara umum, sebagai peninjau, apakah boleh mengatakan "kode ini cacat karena ..., lakukan secara berbeda" atau apakah Anda harus menemukan solusi spesifik?

Sebagai peninjau, tugas Anda adalah memeriksa apakah sepotong kode (atau dokumen) memenuhi tujuan tertentu yang telah disepakati sebelum peninjauan.

Beberapa tujuan ini biasanya akan melibatkan panggilan penilaian apakah tujuan telah terpenuhi atau tidak. Sebagai contoh, tujuan bahwa kode harus dipelihara biasanya memerlukan panggilan penilaian.

Sebagai peninjau, adalah tugas Anda untuk menunjukkan di mana tujuan belum terpenuhi dan itu adalah tugas penulis untuk memastikan bahwa karyanya benar-benar memenuhi tujuan. Dengan cara ini, bukan tugas Anda untuk memberi tahu bagaimana koreksi harus dilakukan.

Di sisi lain, hanya memberi tahu penulis "ini cacat. Perbaiki" biasanya tidak mengarah ke suasana positif dalam tim. Untuk suasana yang positif, ada baiknya untuk setidaknya menunjukkan mengapa ada sesuatu yang salah di mata Anda dan untuk memberikan alternatif yang lebih baik jika Anda memilikinya.
Selain itu, jika Anda meninjau sesuatu yang terlihat "salah" tetapi Anda tidak benar-benar memiliki alternatif yang lebih baik, maka Anda juga dapat meninggalkan komentar di sepanjang baris "Kode / desain ini tidak cocok dengan saya, tapi saya tidak memiliki alternatif yang jelas. Bisakah kita membahas ini? " dan kemudian mencoba untuk mendapatkan sesuatu yang lebih baik bersama.

Beberapa jawaban bagus di sini, tapi saya pikir satu poin penting tidak ada. Itu membuat perbedaan besar yang kode Anda tinjau, seberapa berpengalaman orang itu dan bagaimana dia menangani saran tersebut. Jika Anda mengenal rekan setim Anda dengan baik dan Anda mengharapkan catatan seperti "kode ini cacat karena ..., lakukanlah secara berbeda" menjadi cukup baginya untuk menghasilkan solusi yang lebih baik, maka komentar semacam itu bisa baik-baik saja. Tetapi pasti ada orang-orang di mana komentar semacam itu tidak cukup, dan yang perlu diberi tahu dengan tepat bagaimana memperbaiki kode mereka. Jadi IMHO ini adalah panggilan penilaian yang hanya bisa Anda buat untuk kasus individu.

Jadi secara umum, sebagai peninjau, apakah boleh mengatakan "kode ini cacat, lakukan dengan cara yang berbeda" atau apakah Anda harus menemukan solusi spesifik?

Tak satu pun dari keduanya adalah IMO ideal. Hal terbaik untuk dilakukan adalah berbicara dengan penulis dan memperbaiki masalah secara kolaboratif.

Ulasan kode tidak harus asinkron. Banyak masalah akan terbuka jika Anda berhenti melihatnya sebagai proses birokrasi dan meluangkan sedikit waktu untuk komunikasi langsung.

Dalam ulasan kode, haruskah pengulas selalu menyajikan solusi untuk masalah?

Tidak. Jika Anda melakukan itu bukan resensi, Anda adalah pembuat kode berikutnya.

Dalam ulasan kode, haruskah pengulas tidak pernah menyajikan solusi untuk masalah?

Tidak. Tugas Anda adalah untuk mengomunikasikan masalah yang ada. Jika menghadirkan solusi membuat masalah menjadi jelas maka lakukanlah. Tapi jangan berharap saya mengikuti solusi Anda. Satu-satunya hal yang dapat Anda lakukan di sini adalah membuat suatu poin. Anda tidak bisa mendikte implementasi.

Kapan seorang reviewer harus memberikan solusi untuk masalah?

Saat itulah cara paling efektif untuk berkomunikasi. Kami kode monyet bukan jurusan bahasa Inggris. Kadang-kadang cara terbaik untuk menunjukkan bahwa kode menyebalkan ... kurang optimal ... adalah menunjukkan kepada mereka kode yang menyebalkan kurang ... lebih memilih ... oh neraka Anda tahu maksud saya.

Masalah utama adalah jika orang tahu cara menulis kode dengan lebih baik, biasanya mereka akan melakukannya sejak awal. Apakah kritik cukup spesifik tergantung banyak pada pengalaman penulis. Pemrogram yang sangat berpengalaman mungkin dapat menerima kritik seperti "kelas ini terlalu rumit" dan kembali ke papan gambar dan menghasilkan sesuatu yang lebih baik, karena mereka hanya memiliki hari libur karena sakit kepala atau sedang ceroboh karena mereka terburu-buru.

Namun, biasanya, Anda setidaknya harus mengidentifikasi sumber komplikasi. "Kelas ini terlalu rumit karena melanggar Hukum Demeter di semua tempat." "Kelas ini memadukan lapisan presentasi dan tanggung jawab lapisan kegigihan." Belajar mengidentifikasi alasan-alasan itu dan menjelaskannya secara ringkas adalah bagian dari menjadi pengkaji yang lebih baik. Anda jarang harus masuk ke banyak detail tentang solusi.

Ada dua jenis programmer yang buruk: mereka yang tidak mengikuti praktik standar dan yang "hanya" mengikuti praktik standar.

Ketika saya memiliki kontak kerja yang terbatas / memberikan umpan balik kepada seseorang, saya tidak akan mengatakan, "Ini desain yang buruk." tetapi sesuatu seperti "Bisakah Anda menjelaskan kelas ini kepada saya?" Anda mungkin menemukan itu adalah solusi yang baik, dev sungguh-sungguh melakukan yang terbaik yang dia bisa atau bahkan pengakuan bahwa itu adalah solusi yang buruk, tetapi itu sudah cukup baik.

Bergantung pada responsnya, Anda akan memiliki ide yang lebih baik bagaimana mendekati setiap situasi dan orang. Mereka dapat dengan cepat mengenali masalah dan menemukan perbaikan sendiri. Mereka mungkin meminta bantuan atau hanya akan mencoba pergi dan menyelesaikannya sendiri.

Ada beberapa praktik yang disarankan dalam bisnis kami, tetapi hampir semuanya memiliki pengecualian. Jika Anda memahami proyek dan bagaimana tim mendekati itu, itu bisa menjadi konteks untuk menentukan tujuan tinjauan kode dan bagaimana cara mengatasi masalah.

Saya menyadari ini lebih merupakan pendekatan untuk masalah daripada solusi eksplisit. Akan ada terlalu banyak variabilitas untuk mencakup semua situasi.

Ketika saya meninjau kode saya hanya memberikan solusi untuk masalah yang saya identifikasi jika saya bisa melakukannya dengan sedikit usaha. Saya mencoba untuk lebih spesifik tentang apa yang saya pikir masalahnya, merujuk kembali ke dokumentasi yang ada jika memungkinkan. Mengharapkan peninjau untuk memberikan solusi untuk setiap masalah yang diidentifikasi menciptakan insentif jahat - itu akan mencegah peninjau untuk menunjukkan masalah.

Pendapat saya semakin kuat untuk tidak menyediakan kode dalam sebagian besar kasus, karena beberapa alasan:

• Jika penjelasannya sendiri tidak cukup, mereka selalu dapat meminta sampel dari apa yang Anda pikirkan.
• Anda tidak membuang-buang waktu dengan mencoba membiasakan diri dengan beberapa kode yang belum Anda sentuh dalam waktu yang lama, hanya untuk memodifikasinya sedikit, sementara orang lain baru saja menghabiskan waktu melakukan hal itu.
• Jika mereka sudah terbiasa dengan potongan kode dan Anda tidak, memberikan umpan balik saja dapat menghasilkan kode yang lebih baik daripada yang Anda tulis. Memberi seseorang solusi siap akan sering menyebabkan mereka hanya menggunakannya, tanpa mempertimbangkan untuk memperbaikinya lebih lanjut.
• Selalu memberikan solusi berbatasan dengan merendahkan. Anda bekerja dengan seseorang, semoga karena mereka cukup baik untuk dipekerjakan. Jika Anda berhasil mempelajari mengapa sesuatu adalah ide yang buruk, mengapa mereka tidak mempelajarinya dengan mendengarkan umpan balik dan melakukannya sendiri?
• Selalu memberikan solusi itu aneh. Bayangkan Anda memasangkan program di meja mereka: mereka baru saja menyelesaikan beberapa baris yang menurut Anda tidak bagus. Apakah Anda hanya memberi tahu mereka apa yang Anda temukan dan mengapa, atau apakah Anda mengambil keyboard mereka dan segera menampilkan versi Anda? Inilah yang selalu memberikan solusi seperti apa yang Anda rasakan kepada orang lain.
• Anda selalu dapat mengatakan apa yang akan Anda lakukan sebagai gantinya, tanpa menghabiskan waktu untuk benar-benar menulisnya. Anda melakukan hal itu ketika menjelaskan masalah pertama dalam pertanyaan.
• Jangan membagikan makanan, ajarkan cara memancing;)

Tentu, ada beberapa kasus di mana Anda memikirkan beberapa alternatif spesifik, dan ada baiknya melampirkannya. Tapi itu benar-benar langka dalam pengalaman saya. (banyak ulasan, proyek besar) Penulis asli selalu dapat meminta Anda untuk sampel jika mereka perlu.

Bahkan kemudian, karena alasan ke-3, ketika memberikan sampel mungkin layak dikatakan misalnya "menggunakan x.foo()akan membuat ini lebih sederhana" daripada solusi lengkap - dan biarkan penulis menulisnya. Ini juga menghemat waktu Anda.

Saya pikir kunci untuk ulasan kode adalah untuk menyetujui aturan sebelum ulasan.

Jika Anda memiliki seperangkat aturan yang jelas maka tidak perlu menawarkan solusi. Anda hanya memeriksa bahwa aturan telah diikuti.

Satu-satunya waktu pertanyaan seorang alternatif akan muncul adalah jika pengembang asli tidak dapat memikirkan cara untuk mengimplementasikan fitur yang sesuai dengan aturan. Katakanlah Anda memiliki persyaratan kinerja, tetapi fitur tersebut membutuhkan waktu lebih lama dari ambang Anda setelah beberapa upaya pengoptimalan.

Namun! jika aturan Anda subjektif "Nama harus disetujui oleh saya!" kemudian, ya, Anda baru saja menunjuk diri Anda sebagai ketua dan harus mengharapkan permintaan daftar nama yang dapat diterima.

Contoh warisan (parameter opsional) Anda mungkin lebih baik, int bahwa saya telah melihat aturan peninjauan kode yang melarang metode panjang dan parameter fungsi 'terlalu banyak'. Tapi biasanya ini bisa diselesaikan dengan sepele dengan pemisahan. Itu adalah "solusi ini tampaknya rumit" hal-hal di mana objektivitas Anda mengganggu dan mungkin membutuhkan pembenaran atau solusi alternatif.

Jika solusi potensial cepat dan mudah diketik, saya mencoba memasukkannya dalam komentar peer review saya. Jika tidak, saya paling tidak mencantumkan kekhawatiran saya dan mengapa saya menemukan item tertentu bermasalah. Dalam hal nama variabel / fungsi di mana Anda tidak dapat memikirkan sesuatu yang lebih baik, saya biasanya, mengakui bahwa saya tidak memiliki ide yang lebih baik, dan akhiri komentar dalam bentuk pertanyaan terbuka untuk berjaga-jaga seandainya seseorang dapat . Dengan begitu, jika tidak ada yang memberikan opsi yang lebih baik, ulasan tersebut tidak benar-benar ditahan.

Sebagai contoh Anda berikan dalam pertanyaan Anda, dengan kelas yang dirancang dengan buruk. Saya akan meninggalkan beberapa komentar yang walaupun sepertinya berlebihan, pewarisan mungkin akan menjadi cara terbaik untuk mengatasi masalah yang coba dipecahkan oleh kode, dan biarkan saja. Saya akan mencoba untuk frase dengan cara yang menunjukkan itu bukan show-stopper dan tergantung pada kebijaksanaan pengembang apakah akan memperbaikinya atau tidak. Saya juga akan terbuka dengan pengakuan bahwa Anda tidak terlalu terbiasa dengan bagian kode itu, dan mengundang lebih banyak pengembang dan / atau pengulas yang berpengetahuan luas untuk mengklarifikasi jika ada alasan dilakukannya seperti itu.

Pergi dan bicara dengan orang yang kode yang Anda tinjau. Beri tahu mereka, dengan cara yang bersahabat, bahwa Anda merasa agak sulit untuk dipahami, dan kemudian diskusikan dengan mereka bagaimana hal itu bisa dibuat lebih jelas.

Komunikasi tertulis menyebabkan sejumlah besar waktu terbuang, serta kebencian dan kesalahpahaman.

Tatap muka, bandwidth jauh lebih tinggi, dan seseorang memiliki saluran sisi emosional untuk mencegah permusuhan.

Jika Anda benar-benar berbicara dengan pria itu, itu jauh lebih cepat, dan Anda mungkin membuat teman baru dan menemukan bahwa Anda berdua lebih menikmati pekerjaan Anda.