Apa yang harus dilakukan ketika kode yang dikirimkan untuk tinjauan kode tampaknya terlalu rumit?

Kode ini sulit untuk diikuti tetapi tampaknya (sebagian besar) bekerja dengan baik, setidaknya dengan pengujian yang dangkal. Mungkin ada bug kecil di sana-sini tetapi sangat sulit untuk mengetahui dengan membaca kode jika mereka merupakan gejala dari masalah yang lebih dalam atau perbaikan sederhana. Namun memverifikasi kebenaran keseluruhan secara manual melalui review kode sangat sulit dan memakan waktu, jika mungkin sama sekali.

Apa tindakan terbaik dalam situasi ini? Bersikeras melakukan do-over? Parsial do-over? Mempertimbangkan ulang terlebih dahulu? Perbaiki bug saja dan terima utang teknis ? Lakukan penilaian risiko pada opsi-opsi itu dan kemudian putuskan? Sesuatu yang lain

Jika tidak dapat ditinjau, maka tidak dapat melewati ulasan.

Anda harus memahami bahwa tinjauan kode bukan untuk menemukan bug. Untuk itulah QA diperuntukkan. Tinjauan kode adalah untuk memastikan bahwa pemeliharaan kode di masa mendatang dimungkinkan. Jika Anda bahkan tidak dapat mengikuti kode sekarang, bagaimana Anda bisa dalam enam bulan ketika Anda ditugaskan untuk melakukan peningkatan fitur dan / atau perbaikan bug? Menemukan bug saat ini hanyalah keuntungan sampingan.

Jika terlalu rumit, itu melanggar banyak prinsip SOLID . Refactor, refactor, refactor. Pecah menjadi fungsi-fungsi yang dinamai dengan benar yang melakukan jauh lebih sedikit, lebih sederhana. Anda dapat membersihkannya dan test case Anda akan memastikan bahwa itu terus berfungsi dengan benar. Anda memang memiliki test case, bukan? Jika tidak, Anda harus mulai menambahkannya.

Segala sesuatu yang Anda sebutkan benar-benar valid untuk ditunjukkan dalam tinjauan kode.

Ketika saya menerima ulasan kode, saya melakukan review tes. Jika tes tidak memberikan cakupan yang memadai, itu sesuatu untuk ditunjukkan. Tes harus bermanfaat untuk memastikan bahwa kode berfungsi sebagaimana dimaksud dan akan terus bekerja sebagaimana dimaksud dalam perubahan. Sebenarnya, ini adalah salah satu hal pertama yang saya cari dalam tinjauan kode. Jika Anda belum membuktikan bahwa kode Anda memenuhi persyaratan, saya tidak ingin menginvestasikan waktu saya untuk melihatnya.

Begitu ada tes yang cukup untuk kodenya, jika kodenya rumit atau sulit diikuti, itu juga sesuatu yang harus dilihat manusia. Alat analisis statis dapat menunjukkan beberapa ukuran kompleksitas dan menandai metode yang terlalu rumit serta menemukan kelemahan potensial dalam kode (dan harus dijalankan sebelum tinjauan kode manusia). Tetapi kode dibaca dan dipelihara oleh manusia, dan perlu ditulis untuk pemeliharaannya terlebih dahulu. Hanya jika ada alasan untuk menggunakan kode yang kurang dapat dikelola maka kode tersebut harus ditulis dengan cara itu. Jika Anda perlu memiliki kode yang kompleks atau tidak intuitif, harus didokumentasikan (lebih disukai dalam kode) mengapa kode itu seperti itu dan memiliki komentar yang membantu bagi pengembang masa depan untuk memahami mengapa dan apa yang dilakukan kode.

Idealnya, tolak ulasan kode yang tidak memiliki tes yang sesuai atau memiliki kode yang terlalu rumit tanpa alasan yang bagus. Mungkin ada alasan bisnis untuk maju, dan untuk itu Anda perlu menilai risikonya. Jika Anda terus maju dengan utang teknis dalam kode, segera masukkan tiket ke sistem pelacakan bug Anda dengan beberapa perincian tentang apa yang perlu diubah dan beberapa saran untuk mengubahnya.

Namun memverifikasi kebenaran keseluruhan secara manual melalui review kode sangat sulit dan memakan waktu, jika mungkin sama sekali.

Itu bukan titik review kode. Cara untuk memikirkan tinjauan kode adalah membayangkan ada bug dalam kode dan Anda harus memperbaikinya. Dengan pola pikir ini, telusuri kode (terutama komentar) dan tanyakan pada diri sendiri "Apakah mudah untuk memahami gambaran besar dari apa yang terjadi sehingga saya dapat mempersempit masalahnya?" Jika demikian, ini adalah izin. Kalau tidak, itu gagal. Diperlukan lebih banyak dokumentasi paling tidak, atau mungkin refactoring diperlukan untuk membuat kode tersebut dapat dipahami secara wajar.

Penting untuk tidak menjadi perfeksionis tentang hal itu kecuali jika Anda yakin itulah yang diinginkan majikan Anda. Sebagian besar kode sangat menyebalkan sehingga dapat dengan mudah di-refactor 10 kali berturut-turut, sehingga lebih mudah dibaca setiap kali. Tetapi majikan Anda mungkin tidak mau membayar untuk memiliki kode yang paling mudah dibaca di dunia.

Namun memverifikasi kebenaran keseluruhan secara manual melalui review kode sangat sulit dan memakan waktu, jika mungkin sama sekali.

Bertahun-tahun yang lalu, sebenarnya adalah tugas saya untuk melakukan hal itu dengan menilai pekerjaan rumah siswa. Dan sementara banyak yang memberikan kualitas yang masuk akal dengan bug di sana-sini, ada dua yang menonjol. Keduanya selalu mengirimkan kode yang tidak memiliki bug. Satu kode yang dikirimkan dapat saya baca dari atas dan bawah dengan kecepatan tinggi dan tandai 100% benar tanpa usaha. Kode yang dikirimkan lain adalah satu WTF setelah yang lain, tetapi entah bagaimana berhasil menghindari bug. Rasa sakit yang mutlak untuk ditandai.

Hari ini kode kedua akan ditolak kodenya dalam tinjauan kode. Jika memverifikasi kebenaran sangat sulit dan memakan waktu, maka itu adalah masalah dengan kode. Seorang programmer yang baik akan mencari tahu bagaimana menyelesaikan masalah (membutuhkan waktu X) dan sebelum memberikannya ke refactor review kode sehingga tidak hanya melakukan pekerjaan, tetapi jelas melakukan pekerjaan. Itu membutuhkan waktu kurang dari X dalam waktu dan menghemat banyak waktu di masa depan. Seringkali dengan mengungkap bug bahkan sebelum mereka pergi ke tahap review kode. Selanjutnya dengan membuat review kode jauh lebih cepat. Dan sepanjang waktu di masa depan dengan membuat kode lebih mudah diadaptasi.

Jawaban lain mengatakan bahwa kode beberapa orang dapat di refactored 10 kali, menjadi lebih mudah dibaca setiap kali. Itu menyedihkan. Itu pengembang yang harus mencari pekerjaan yang berbeda.

Apakah kode lama ini yang diubah sedikit? (100 baris kode diubah dalam basis kode 10.000 baris masih sedikit perubahan) Kadang-kadang ada kendala waktu dan pengembang dipaksa untuk tetap berada dalam kerangka kerja lama dan tidak nyaman, hanya karena penulisan ulang lengkap akan memakan waktu lebih lama dan jauh dari anggaran . + Biasanya ada risiko yang terlibat, yang dapat menelan biaya jutaan dolar ketika dinilai secara tidak benar. Jika ini kode lama, dalam kebanyakan kasus Anda harus menjalaninya. Jika Anda tidak memahaminya sendiri, bicarakan dengan mereka, dan dengarkan apa yang mereka katakan, cobalah untuk memahaminya. Ingat, mungkin sulit untuk mengikuti untuk Anda, tetapi baik-baik saja untuk orang lain. Ambil sisi mereka, lihat dari ujung mereka.

Apakah ini kode baru ? Tergantung pada batasan waktu, Anda harus mengadvokasi untuk refactor sebanyak mungkin. Apakah boleh untuk menghabiskan lebih banyak waktu pada ulasan kode jika perlu. Anda tidak boleh timebox sendiri sampai 15 menit, dapatkan ide dan lanjutkan. Jika penulis menghabiskan waktu seminggu untuk menulis sesuatu, tidak apa-apa menghabiskan 4-8 jam untuk memeriksanya. Tujuan Anda di sini adalah untuk membantu mereka melakukan refactor. Anda tidak hanya mengembalikan kode yang mengatakan "refactor. Now". Lihat metode mana yang dapat diuraikan, cobalah untuk menghasilkan ide untuk memperkenalkan kelas baru, dll.

Seringkali, patch / changelists "rumit" adalah yang melakukan banyak hal berbeda sekaligus. Ada kode baru, kode dihapus, kode refactored, kode pindah, tes diperluas; sulit untuk melihat gambaran besarnya.

Petunjuk umum adalah bahwa tambalan itu besar tetapi uraiannya kecil: "Terapkan $ FOO."

Cara yang masuk akal untuk menangani tambalan semacam itu adalah dengan meminta petak itu dipecah menjadi serangkaian potongan yang lebih kecil dan mandiri. Sama seperti prinsip tanggung jawab tunggal mengatakan suatu fungsi harus melakukan hanya satu hal, sebuah tambalan juga harus fokus pada satu hal.

Sebagai contoh, tambalan pertama mungkin berisi refactoring murni mekanis yang tidak membuat perubahan fungsional, dan tambalan terakhir dapat fokus pada implementasi aktual dan pengujian $ FOO dengan lebih sedikit gangguan dan ikan haring merah.

Untuk fungsionalitas yang membutuhkan banyak kode baru, kode baru sering kali dapat diperkenalkan dalam potongan yang dapat diuji yang tidak mengubah perilaku produk sampai tambalan terakhir dalam seri benar-benar memanggil kode baru (flip bendera).

Adapun untuk melakukan ini dengan bijaksana, saya biasanya mengatakan itu sebagai masalah saya dan kemudian meminta bantuan penulis: "Saya mengalami kesulitan mengikuti semua yang terjadi di sini. Bisakah Anda memecah tambalan ini menjadi langkah-langkah kecil untuk membantu saya memahami bagaimana semua ini cocok bersama?" Terkadang perlu membuat saran khusus untuk langkah-langkah kecil.

Jadi tambalan besar seperti "Implement $ FOO" berubah menjadi serangkaian tambalan seperti:

1. Memperkenalkan versi baru Frobnicate yang membutuhkan sepasang iterator karena saya akan perlu menyebutnya dengan urutan selain vektor untuk mengimplementasikan $ FOO.
2. Ganti semua penelepon Frobnicate yang ada untuk menggunakan versi baru.
3. Hapus Frobnicate lama.
4. Frobnicate melakukan terlalu banyak. Faktor langkah refrumple ke dalam metode sendiri dan tambahkan tes untuk itu.
5. Perkenalkan Zerzify, dengan tes. Belum digunakan, tetapi saya akan membutuhkannya untuk $ FOO.
6. Terapkan $ FOO dalam hal Zerzify dan Frobnicate baru.

Perhatikan bahwa langkah 1-5 tidak membuat perubahan fungsional pada produk. Mereka sepele untuk ditinjau, termasuk memastikan bahwa Anda memiliki semua tes yang tepat. Bahkan jika langkah 6 masih "rumit," setidaknya itu berfokus pada $ FOO. Dan log secara alami memberi Anda ide yang lebih baik tentang bagaimana $ FOO diterapkan (dan mengapa Frobnicate diubah).

Seperti yang ditunjukkan orang lain, tinjauan kode tidak benar-benar dirancang untuk menemukan bug. Jika Anda menemukan bug selama peninjauan kode yang mungkin berarti Anda tidak memiliki cakupan pengujian otomatis yang cukup (mis. Pengujian unit / integrasi). Jika tidak ada cakupan yang cukup untuk meyakinkan saya bahwa kode melakukan apa yang seharusnya , saya biasanya meminta tes lebih lanjut dan menunjukkan jenis kasus pengujian yang saya cari dan biasanya tidak mengizinkan kode ke basis kode yang tidak memiliki cakupan yang memadai .

Jika arsitektur tingkat tinggi terlalu kompleks atau tidak masuk akal, saya biasanya mengadakan pertemuan dengan beberapa anggota tim untuk membicarakannya. Terkadang sulit untuk beralih pada arsitektur yang buruk. Jika dev adalah seorang pemula, saya biasanya memastikan kita melewati apa yang mereka pikirkan sebelumnya daripada bereaksi terhadap permintaan tarikan yang buruk. Ini biasanya benar bahkan dengan pengembang yang lebih berpengalaman jika masalahnya tidak memiliki solusi jelas yang kemungkinan besar akan diambil.

Jika kompleksitas diisolasi ke tingkat metode yang biasanya dapat diperbaiki secara iteratif dan dengan tes otomatis yang baik.

Satu poin terakhir. Sebagai peninjau Anda perlu memutuskan apakah kompleksitas kode disebabkan oleh kompleksitas esensial atau tidak disengaja . Kompleksitas esensial berkaitan dengan bagian-bagian perangkat lunak yang secara sah sulit dipecahkan. Kompleksitas tidak disengaja mengacu pada semua bagian lain dari kode yang kita tulis yang terlalu rumit tanpa alasan dan dapat dengan mudah disederhanakan.

Saya biasanya memastikan bahwa kode dengan kompleksitas esensial adalah benar-benar dan tidak dapat disederhanakan. Saya juga bertujuan untuk lebih banyak cakupan tes dan dokumentasi yang baik untuk bagian-bagian ini. Kompleksitas tak disengaja hampir selalu harus dibersihkan selama proses permintaan tarik karena itu adalah sebagian besar kode yang kita tangani dan dapat dengan mudah menyebabkan mimpi buruk pemeliharaan bahkan dalam jangka pendek.

Seperti apa tesnya? Mereka harus jelas, sederhana, dan mudah dibaca dengan idealnya hanya satu menegaskan. Tes harus secara jelas mendokumentasikan perilaku yang dimaksud dan menggunakan kasus kode.

Jika tidak diuji dengan baik, itu adalah tempat yang baik untuk mulai meninjau.