Apa yang Anda katakan dalam ulasan kode ketika orang lain membangun solusi yang rumit? [Tutup]

Suatu hari saya meninjau kode seseorang di tim saya menulis. Solusinya tidak sepenuhnya berfungsi dan desainnya jauh lebih rumit - yang berarti menyimpan informasi yang tidak perlu, membangun fitur yang tidak perlu, dan pada dasarnya kode memiliki banyak kerumitan yang tidak perlu seperti pelapisan emas dan mencoba memecahkan masalah yang tidak ada.

Dalam situasi ini saya bertanya, "Mengapa itu dilakukan seperti ini?"

Jawabannya adalah orang lain merasa ingin melakukannya dengan cara itu.

Lalu saya bertanya apakah salah satu dari fitur ini adalah bagian dari spesifikasi proyek, atau apakah ada gunanya bagi pengguna akhir, atau apakah ada data tambahan yang akan disajikan kepada pengguna akhir.

Jawabannya adalah tidak.

Jadi saya menyarankan agar dia menghapus semua kompleksitas yang tidak perlu. Jawaban yang biasanya saya dapatkan adalah "sudah selesai".

Pandangan saya adalah bahwa itu tidak dilakukan, itu buggy, tidak melakukan apa yang diinginkan pengguna, dan biaya perawatan akan lebih tinggi daripada jika dilakukan dengan cara yang lebih sederhana yang saya sarankan.

Skenario yang setara adalah:
Rekan menghabiskan 8 jam kode refactoring dengan tangan yang bisa dilakukan secara otomatis di Resharper dalam 10 detik. Secara alami saya tidak mempercayai refactoring dengan tangan karena kualitasnya meragukan dan tidak sepenuhnya diuji.
Lagi-lagi respons yang saya dapatkan adalah "sudah selesai."

Apa tanggapan yang pantas untuk sikap ini?

Mentalitas / sikap

• Menurut contoh
• Beri peringatan secara pribadi (satu-ke-satu, di luar tinjauan kode)
• Dorong mentalitas Keep-it-simple di antara anggota tim

Manajemen tim

• Luangkan lebih banyak waktu pada spesifikasi item kerja (seperti arsitektur, garis besar algoritma, gambar rangka UI, dll)
• Dorong anggota tim untuk mencari klarifikasi tentang ruang lingkup item pekerjaan
• Dorong anggota tim untuk membahas cara menerapkan item kerja
• Buat perkiraan yang masuk akal untuk setiap item pekerjaan sebelum memulai, dan lakukan upaya terbaik untuk mencapainya
• Pantau "peningkatan" anggota tim.
  • Setelah dinasihati atau ditunjukkan cara yang benar untuk melakukan sesuatu, lihat apakah anggota tim membaik.

Tingkat keahlian

• Alokasikan waktu untuk sesi pemrograman pasangan atau sesi pelatihan satu-ke-satu untuk memanfaatkan alat pengembang sebaik-baiknya (refactoring, tinjauan kode)

Manajemen proyek (risiko)

• Melakukan tinjauan kode lebih sering, secara asinkron (Catatan)
  • Catatan tentang "tidak sinkron"
    • Peninjau kode harus mendapat pemberitahuan / undangan untuk meninjau perubahan segera setelah dilakukan
    • Peninjau kode harus memiliki kesempatan untuk meninjau kode sebelum pertemuan dengan pengembang.
    • Jika diperlukan klarifikasi dari pengembang, lakukan secara informal di IM / email tanpa memberikan pendapat negatif

Apa yang Anda katakan dalam ulasan kode ketika orang lain membangun solusi yang rumit?

Anda berkata: "Anda membangun solusi yang terlalu rumit."

Jadi saya menyarankan agar dia menghapus semua kompleksitas yang tidak perlu. Jawaban yang biasanya saya dapatkan adalah "sudah selesai."

Jika sudah terlambat untuk mengubah apa pun, mengapa Anda melakukan tinjauan kode?

"Sudah dilakukan" bukan jawaban yang memuaskan. Selesai berarti teruji dan berfungsi. Setiap kode tambahan yang tidak melakukan sesuatu yang bermanfaat harus dipertahankan dengan cara yang benar (dihapus).

Tugaskan dia lagi untuk meminta refactor dan mengoptimalkan solusinya. Jika dia tidak melakukan itu, tetapkan dia seorang programmer berpasangan dan berharap dia akan belajar sesuatu dari rekannya.

Jadi saya menyarankan agar dia menghapus semua kompleksitas yang tidak perlu. Jawaban yang biasanya saya dapatkan adalah "sudah selesai".

Itu bukan jawaban yang bisa diterima:

• Jika benar - benar terlambat untuk berubah, maka tinjauan kode sebagian besar membuang-buang waktu, dan manajemen perlu mengetahui hal ini.

• Jika itu benar-benar cara untuk mengatakan "Saya tidak ingin berubah", maka Anda perlu mengambil posisi bahwa kompleksitas tambahannya BURUK untuk basis kode KARENA masalah / biaya yang harus dikeluarkan kemudian. Dan mengurangi potensi masalah di masa depan alasan sebenarnya Anda melakukan peninjauan kode di tempat pertama.

Dan ...

... solusinya tidak berfungsi penuh ...

Itu sangat mungkin akibat langsung dari kompleksitas yang tidak perlu. Programmer telah membuatnya sangat kompleks sehingga dia tidak lagi sepenuhnya memahaminya dan / atau dia telah membuang-buang waktu untuk mengimplementasikan kompleksitasnya daripada titik-titik fungsi. Akan bermanfaat untuk menunjukkan kepada programmer bahwa memotong kompleksitas sebenarnya dapat membawanya ke program kerja lebih cepat.

Sekarang, sepertinya Anda tidak memiliki kekuatan (atau mungkin kepercayaan diri) untuk "mendorong balik keras" dalam hal ini. Namun demikian, perlu membuat sedikit suara tentang ini (tanpa mempersonalisasikannya) dengan harapan bahwa pembuat kode yang menyinggung akan melakukan pekerjaan yang lebih baik ... lain kali.

Apa tanggapan yang pantas untuk sikap ini?

Pada akhirnya, bawalah ke perhatian manajemen ... kecuali Anda memiliki kekuatan untuk memperbaikinya sendiri. (Tentu saja, ini tidak akan membuatmu populer.)

Anda benar, mereka salah:

• Prinsip YAGNI rusak
• Prinsip KISS rusak
• apakah kode sepenuhnya diuji? Jika tidak, maka itu tidak dilakukan

Apa tanggapan yang pantas untuk sikap ini?

Lakukan review kode yang tepat. Jika mereka menolak untuk menerapkan perubahan yang disarankan tanpa alasan, maka berhentilah membuang waktu Anda satu ulasan kode. Anda juga dapat mengeskalasi masalahnya ke bos mereka .

Salah satu tindakan yang diambil oleh tim kami, yang secara dramatis memperbaiki situasi dalam kasus-kasus seperti itu, adalah pindah ke perubahan yang jauh lebih kecil .

Alih-alih mengerjakan satu tugas selama satu hari atau lebih dan kemudian melakukan tinjauan kode (besar), kami mencoba untuk lebih sering checkin (hingga 10 kali sehari). Tentu saja ini juga memiliki beberapa kelemahan, misalnya resensi perlu sangat responsif, yang mengurangi output sendiri (karena seringnya gangguan).

Keuntungannya adalah, bahwa masalah terdeteksi dan dapat dipecahkan lebih awal, sebelum sejumlah besar pekerjaan dengan cara yang salah dilakukan.

Anda harus fokus pada akar penyebab masalah:

1. Pendidikan programmer berfokus pada peningkatan kompleksitas yang diberikan kepada programmer. Kemampuan untuk melakukan ini diuji oleh sekolah. Jadi banyak programmer akan berpikir bahwa jika mereka menerapkan solusi sederhana, mereka tidak melakukan pekerjaan mereka dengan benar.
2. Jika programmer mengikuti pola yang sama dengan yang telah ia lakukan ratusan kali saat berada di universitas, itu hanya cara berpikir para programmer - lebih banyak kompleksitas lebih menantang dan karenanya lebih baik.
3. Jadi untuk memperbaikinya Anda harus menjaga pemisahan ketat dari apa persyaratan perusahaan Anda relatif terhadap kompleksitas dibandingkan dengan apa yang biasanya diperlukan dalam pendidikan programmer. Rencana yang baik adalah aturan seperti "tingkat kompleksitas tertinggi harus disediakan hanya untuk tugas yang dirancang untuk meningkatkan keterampilan Anda - dan itu tidak boleh digunakan dalam kode produksi".
4. Ini akan menjadi kejutan bagi banyak programmer bahwa mereka tidak diizinkan untuk melakukan desain paling gila mereka di lingkungan kode produksi yang penting. Cadangan saja waktu untuk programmer untuk melakukan desain eksperimental, dan kemudian menyimpan semua kompleksitas di sisi pagar.

(dalam ulasan kode sudah terlambat untuk mengubahnya)

Saya tidak tahu apa pun yang berfungsi setelah kode ditulis.

Sebelum kode ini ditulis, orang dapat mendiskusikan cara alternatif untuk melakukannya. Kuncinya adalah menyumbangkan ide satu sama lain, jadi semoga yang masuk akal akan dipilih.

Ada pendekatan lain yang bekerja dengan kontraktor - kontrak harga tetap. Semakin sederhana solusinya, semakin banyak $$ yang harus dipertahankan programmer.

Anda tidak dapat memperbaiki dunia.

Anda bahkan tidak dapat memperbaiki semua kode pada proyek Anda. Anda mungkin tidak dapat memperbaiki praktik pengembangan pada proyek Anda, setidaknya tidak bulan ini.

Sayangnya, apa yang Anda alami dalam ulasan kode terlalu umum. Saya telah bekerja di beberapa organisasi di mana saya sering menemukan diri saya meninjau 100 baris kode yang bisa ditulis dalam sepuluh, dan saya mendapat jawaban yang sama dengan yang Anda lakukan: "Sudah ditulis dan diuji" atau "Kami sedang mencari bug, bukan desain ulang. "

Adalah fakta bahwa beberapa kolega Anda tidak dapat memprogram sebaik yang Anda bisa. Beberapa dari mereka mungkin sangat buruk dalam hal itu. Jangan khawatir tentang itu. Beberapa kelas dengan penerapan buruk tidak akan menurunkan proyek. Sebaliknya, fokuslah pada bagian-bagian pekerjaan mereka yang akan memengaruhi orang lain. Apakah tes unit memadai (jika Anda memilikinya)? Apakah antarmuka dapat digunakan? Apakah ini didokumentasikan?

Jika antarmuka ke kode buruk ok, jangan khawatir tentang hal itu sampai Anda harus memeliharanya, kemudian menulis ulang. Jika ada yang mengeluh, sebut saja itu refactoring. Jika masih mengeluh, cari posisi di organisasi yang lebih canggih.

Harus ada kebijakan standar dalam proyek yang mengontrol prosedur pemeriksaan kualitas dan alat yang digunakan.

Orang harus tahu apa yang harus mereka lakukan dan alat apa yang diterima untuk digunakan dalam proyek ini.

Jika Anda belum melakukan ini, atur pikiran Anda dan lakukanlah.

Tinjauan kode harus memiliki daftar periksa item standar. Jika Anda "sudah selesai" dan tidak, maka secara pribadi, saya tidak ingin bertanggung jawab atas pekerjaan pengembang ini sebagai manajer proyek atau pengembang senior. Sikap ini tidak boleh ditoleransi. Saya bisa memahami berdebat tentang bagaimana melakukan sesuatu atau bahkan segala sesuatu, tetapi begitu solusi diterima, berbohong tidak boleh ditoleransi dan itu harus dinyatakan dengan jelas.

Toko Anda perlu menerapkan beberapa metodologi desain.

• Persyaratan harus didefinisikan dengan jelas.
• Anda perlu mengembangkan kasus penggunaan yang mendukung persyaratan.
• Anda perlu menentukan fungsi yang diperlukan untuk mengimplementasikan kasus penggunaan.
• Anda perlu menentukan persyaratan non-fungsional (waktu respons, ketersediaan, dll.)
• Anda memerlukan RTM (Requiements Tracabilty Matrix) untuk memetakan setiap fungsi sistem kembali ke use case dan persyaratan nyata.
• Jatuhkan fungsi apa pun yang tidak mendukung persyaratan aktual.
• Terakhir di ulasan kode Anda menandai kode apa pun yang tidak secara langsung menerapkan atau mendukung fungsi yang ditetapkan.

Mungkin tidak terlalu rumit karena itu membuat kebanyakan orang merasa buruk sesudahnya. Saya berasumsi bahwa ketika ini terjadi sudah banyak kode telah ditulis tanpa berbicara sepatah kata pun tentang hal itu. (Kenapa begitu? Karena orang itu memiliki cukup otoritas sehingga kodenya tidak harus ditinjau dalam kenyataan?)

Kalau tidak, saya kira membuat tinjauan kode kurang formal tetapi lebih sering. Dan sebelum menulis modul besar mungkin Anda harus dengan cepat mendiskusikan pendekatan apa yang harus diambil.

Mengatakan "ini terlalu rumit" membuat Anda tidak berhasil.

Sangat disayangkan, tetapi Code Review, sering kali, lebih banyak untuk masa depan daripada untuk saat ini. Terutama di lingkungan perusahaan / perusahaan, kode yang dikirim selalu lebih berharga daripada kode yang tidak dikirim.

Ini, tentu saja, tergantung pada kapan ulasan kode selesai. Jika itu bagian dari proses pengembangan, maka Anda bisa mendapatkan manfaat saat ini. Tetapi jika CR dianggap lebih sebagai post-mortem, lebih baik Anda menunjukkan apa yang bisa dilakukan dengan lebih baik di masa depan. Dalam kasus Anda (seperti yang dikatakan orang lain), tunjukkan YAGNI dan KISS secara umum, dan mungkin beberapa area spesifik di mana prinsip-prinsip ini dapat diterapkan.

Apa artinya terlalu rumit? Anda membuat pernyataan ambigu maka Anda akan mendapatkan jawaban ambigu / tidak memuaskan sebagai tanggapan. Apa yang terlalu rumit bagi satu orang sempurna bagi orang lain.

Tujuan ulasan adalah untuk menunjukkan masalah dan kesalahan tertentu, bukan untuk mengatakan Anda tidak menyukainya, yang menyiratkan pernyataan "terlalu rumit".

Jika Anda melihat masalah (terlalu rumit) maka ucapkan sesuatu yang lebih konkret seperti:

• Tidakkah mengubah bagian X ke Y menyederhanakan kode atau membuatnya lebih mudah dipahami?
• Saya tidak mengerti apa yang Anda lakukan di sini di bagian X, saya pikir apa yang Anda coba lakukan adalah ini. Sajikan cara yang lebih bersih untuk melakukannya.
• Bagaimana Anda menguji ini? Apakah Anda menguji ini? Jika terlalu rumit ini biasanya akan menyebabkan tatapan kosong. Meminta tes akan sering membuat orang menyederhanakan sendiri kode mereka ketika mereka tidak tahu bagaimana cara menguji kode asli.
• Tampaknya ada kesalahan di sini, mengubah kode ini akan memperbaiki masalah.

Siapa saja dapat menunjukkan masalah, terutama yang ambigu. Ada subset yang jauh lebih kecil yang dapat menyajikan solusi. Komentar ulasan Anda harus sespesifik mungkin. Mengatakan sesuatu itu terlalu rumit tidak berarti banyak, itu bahkan dapat menyebabkan orang lain menganggap ANDA tidak kompeten karena tidak dapat memahami kode. Perlu diingat bahwa sebagian besar pengembang tidak memiliki petunjuk tentang perbedaan antara desain yang baik atau buruk.

Kadang-kadang layak sebagai kelompok untuk fokus pada beberapa prinsip "Agile" - mereka dapat membantu kelompok atau individu yang tampaknya sedikit keluar jalur.

Fokus tidak harus berarti kerja ulang tim Anda yang hebat, tetapi Anda semua harus duduk dan membahas praktik apa yang paling penting bagi Anda sebagai sebuah tim. Saya sarankan mendiskusikan setidaknya ini (dan mungkin beberapa):

• Apakah hal paling sederhana yang mungkin bisa berhasil?
• Anda tidak akan membutuhkannya (apakah Anda memecahkan masalah yang tidak ada dalam spesifikasi)
• Tulis tes sebelum pengkodean (Membantu Anda memfokuskan kode Anda)
• Jangan ulangi dirimu sendiri

Juga ulasan berkala (Mingguan?) Tentang apa yang berhasil, apa yang tidak dan apa yang masih dibutuhkan bisa sangat membantu ... Jika tidak ada yang lain, mengapa tidak berkomitmen untuk satu jam seminggu untuk membahas nilai-nilai dan praktik tim?

Peningkatan, jika Anda memiliki manajer yang berpikiran teknis. Ini terdengar seperti kebiasaan yang perlu dihilangkan.

Jika kode tidak dibuat untuk spesifikasi, maka menurut definisi harus gagal tinjauan kode. Saya tidak mengerti konsep "kita telah melakukan sesuatu yang tidak ada yang meminta, dan itu tidak berfungsi sehingga kita akan membiarkannya di sana daripada melakukan sesuatu yang diminta seseorang untuk berhasil".

Ini adalah kebiasaan buruk bagi pengembang mana pun. Jika dia bekerja dengan spesifikasi desain maka tidak cocok tanpa alasan yang baik adalah tidak, tidak.

Satu kata: gesit

Itu tentu tidak menyelesaikan segalanya. Tetapi dengan memerintah dalam iterasi Anda (1-2 minggu, misalnya), membatasi pekerjaan yang sedang berjalan dan meningkatkan perencanaan / tinjauan sprint, Anda harus menghindari kesalahan seperti air terjun ini. Anda perlu visibilitas yang lebih baik ke dalam apa yang sebenarnya dilakukan - saat ini sedang dilakukan.

Untuk pengembangan berbasis proyek yang normal, saya akan merekomendasikan mengadopsi pendekatan Scrum . Untuk lingkungan pengembangan / integrasi berkelanjutan, dan terutama jika Anda memiliki banyak pengembang yang mengerjakan proyek yang sama atau terkait, pertimbangkan untuk memasukkan unsur-unsur Kanban . Pendekatan lain yang efektif adalah untuk meningkatkan pemrograman pasangan , suatu praktik yang didefinisikan pemrograman Ekstrim .

Situasi Anda hampir tidak unik. Dan bahkan dengan tim kecil, proses bisa sangat membantu untuk menghindari situasi Anda sekarang. Dengan visibilitas yang tepat dan jaminan yang cukup terawat, pertanyaan seperti ini menjadi keputusan perencanaan sprint - menyelamatkan Anda dari mengelola utang teknis .

Apa yang saya katakan di masa lalu adalah "kode ini rumit dan saya tidak yakin dengan apa yang coba dilakukan, apakah mungkin untuk menyederhanakan atau menulisnya dengan lebih jelas?"

Anda harus mengkodekan setelah menghapus / memutar kembali kode mereka: "Ups, kode Anda sudah hilang. Harap tulis ulang. Seperti yang sudah Anda tulis, Anda hanya perlu kurang dari dua puluh menit untuk memberikan HANYA kode yang diperlukan oleh spesifikasi.

"Ulasan saya berikutnya dalam 20 menit.

"Selamat siang."

JANGAN terima argumen apa pun!

Selesai, IMHO

Chris