Apa yang harus saya lakukan sebagai pengulas ketika kode “harus” buruk?

18

Saya bekerja di sebuah proyek yang dirancang dengan buruk, rekayasa ulang di mana ulasan kode wajib telah diperkenalkan beberapa bulan yang lalu.

Saya telah diminta untuk meninjau sepotong kode substansial yang mengimplementasikan fitur baru. Ini menanggung kelemahan yang sama dengan sisa basis kode kami. Saya mengerti bahwa sebagian besar, kelemahan itu merayap ke dalam kode baru, misalnya. dengan harus mewarisi dari kelas yang dirancang dengan buruk atau mengimplementasikan antarmuka yang dirancang dengan buruk sambil mempertahankan kompatibilitas, dan saya tidak bisa benar-benar menawarkan solusi yang jauh lebih baik yang tidak akan melibatkan penulisan ulang setengah dari basis kode. Tapi saya merasa bahwa dari sudut pandang teknik, tidak ada gunanya untuk basis kode yang sudah rusak bahwa kita bahkan melanggar lebih dari itu. Kode yang saya ulas pasti buruk, tetapi harus jika fitur tersebut ingin diterapkan.

Bagaimana saya harus bersikap sehubungan dengan ulasan khusus ini? Apakah ada cara bagi saya untuk mempertahankan integritas dan tetap konstruktif?

Harap perhatikan bahwa saya bertanya tentang batasan tinjauan kode dalam konteks ini. Saya menyadari bahwa masalahnya disebabkan oleh faktor-faktor lain termasuk organisasi dan budaya kerja, tetapi yang ingin saya ketahui adalah bagaimana menangani ulasan itu sendiri.

Merah
sumber
1
Apakah dokumen pengembang mengapa itu dilakukan seperti ini dan bahkan mungkin menulis masalah untuk masalah inti pada pelacak masalah?
Luc Franken
Sangat jarang, dan tidak.
Merah
4
Apakah ada selera dalam organisasi Anda untuk melakukan sesuatu tentang hutang teknis?
Robert Harvey
5
Jika kodenya tidak salah mengingat lingkungan saya tidak akan menyerang kodenya. Sebagian besar sejak Anda menulis Anda tidak melihat peningkatan nyata yang mungkin terjadi saat ini. Itu akan menciptakan konflik tanpa peluang untuk meningkat. Apa yang akan saya lakukan adalah mengajar untuk mendokumentasikan dengan lebih baik sebagai langkah pertama. Kedua, buat prosedur agar mereka menulis masalah yang jelas. Itu akan menghasilkan 2 hal: dokumentasi tentang masalah sehingga yang berikutnya dapat bekerja lebih cepat. Dan di samping itu Anda mendapatkan daftar masalah konkret yang berpotensi diperbaiki. Saya suka GitHub menyebutkan karena Anda melihat berapa kali itu berulang jika mereka menandainya.
Luc Franken

Jawaban:

25

Sederhana:

1. Dokumentasikan hutang teknis Anda

Anda telah mengidentifikasi sepotong kode yang berfungsi tetapi memiliki beberapa masalah teknis dengannya. Ini utang teknis . Seperti jenis utang lainnya, kondisi ini akan semakin memburuk jika tidak ditangani.

Langkah pertama dalam menangani utang teknis ini adalah mendokumentasikannya. Lakukan ini dengan menambahkan item dalam pelacak masalah Anda yang menggambarkan utang. Ini akan membantu Anda mendapatkan gambaran yang lebih jelas tentang besarnya masalah dan juga akan membantu menyusun rencana untuk mengatasinya.

2. Secara bertahap membayar kembali utang Anda

Ubah proses pengembangan perangkat lunak Anda untuk memperhitungkan pembayaran utang teknis. Ini mungkin melibatkan sprint pengerasan sesekali, atau hanya menyelesaikan item utang secara berkala (misalnya, 2 item per minggu). Bagian penting untuk memastikan Anda memotong utang lebih cepat daripada yang timbul (utang, bahkan utang teknis, memiliki bunga).

Setelah Anda mencapai titik di mana Anda tidak lagi memiliki Defisit Teknis, Anda sedang menuju basis kode yang lebih sehat :)

MetaFight
sumber
5

Sebagai catatan tambahan: cari pekerjaan baru. Yang ini tidak akan menjadi lebih baik.

Sasaran kode yang Anda tinjau adalah:

  • Untuk mengirim fitur, yang harus bekerja sesuai dengan persyaratan.

  • Untuk mengurangi pertumbuhan utang teknis.

Tujuan pertama ditinjau dengan memeriksa bahwa tes unit, integrasi, sistem dan fungsional di sini, bahwa mereka relevan, dan bahwa mereka mencakup semua situasi yang harus diuji. Anda juga harus memeriksa keyakinan yang mungkin dimiliki penulis asli tentang bahasa pemrograman, yang dapat menyebabkan bug halus atau kode yang berpura-pura melakukan sesuatu yang berbeda dari apa yang sebenarnya dilakukannya.

Tujuan kedua adalah pertanyaan yang menjadi fokus pertanyaan Anda. Di satu sisi, kode baru tidak diharapkan untuk meningkatkan utang teknis. Di sisi lain, ruang lingkup peninjauan adalah kode itu sendiri, tetapi dalam konteks basis kode keseluruhan. Dari sana, Anda, sebagai peninjau, dapat mengharapkan dua pendekatan dari penulis asli:

  • Kode luar bukan salah saya. Saya hanya mengimplementasikan fitur dan tidak peduli tentang basis kode keseluruhan.

    Dalam perspektif ini, kode akan menyalin kelemahan basis kode, dan dengan demikian meningkatkan hutang teknis: kode yang lebih buruk selalu lebih buruk.

    Walaupun ini adalah pendekatan jangka pendek yang valid, dalam jangka panjang, ini akan menghasilkan peningkatan keterlambatan dan produktivitas yang rendah, dan pada akhirnya menyebabkan proses pengembangan menjadi sangat mahal dan berisiko, sehingga produk akan berhenti berevolusi.

  • Menulis kode baru adalah peluang untuk memperbaiki kode lama.

    Dalam perspektif ini, efek dari cacat kode warisan pada yang baru dapat dibatasi. Selain itu, hutang teknis dapat dikurangi, atau setidaknya tidak meningkat secara proporsional dengan pertumbuhan kode.

    Meskipun ini adalah pendekatan jangka panjang yang valid, ia memiliki risiko jangka pendek. Yang utama adalah bahwa, jangka pendek, kadang - kadang akan membutuhkan lebih banyak waktu untuk mengirim fitur tertentu. Aspek penting lainnya adalah bahwa jika kode warisan tidak diuji, refactoring itu menghadirkan risiko besar untuk memperkenalkan regresi.

Bergantung pada perspektif yang ingin Anda dorong, Anda mungkin cenderung menyarankan orang yang ditinjau untuk lebih atau tidak. Dalam semua kasus, jangan berharap potongan kode yang sempurna dan bersih dengan arsitektur dan desain yang bagus di dalam basis kode yang jelek. Yang tidak boleh Anda dorong adalah perilaku di mana pengembang yang berpengetahuan luas yang harus bekerja pada basis kode jelek mencoba melakukan bagiannya dengan baik. Alih-alih membuat segalanya lebih sederhana, itu hanya membuat mereka lebih rumit dari sebelumnya. Sekarang, alih-alih kode buruk seragam, Anda memiliki bagian dengan pola desain, bagian lain dengan kode bersih, jelas, bagian lain yang secara luas dire-refraktori seiring waktu, dan tidak ada kesatuan apa pun.

Bayangkan, misalnya, bahwa Anda menemukan basis kode warisan dari situs web berukuran sedang. Anda dikejutkan oleh kurangnya struktur yang biasa dan fakta bahwa logging, ketika selesai, dilakukan dengan menambahkan barang ke file teks dengan tangan, alih-alih menggunakan kerangka logging. Anda memutuskan untuk fitur baru menggunakan MVC dan kerangka kerja logging.

Rekan Anda menerapkan fitur lain dan sangat terkejut dengan kurangnya ORM di mana orang akan membuat ukuran yang sempurna. Jadi dia mulai menggunakan ORM.

Baik Anda, maupun kolega Anda tidak dapat melewati ratusan ribu baris kode untuk menggunakan MVC, atau kerangka kerja logging, atau ORM di mana-mana. Sebenarnya, itu akan membutuhkan kerja berbulan-bulan: bayangkan memperkenalkan MVC; berapa lama? Atau bagaimana dengan ORM dalam situasi di mana query SQL dihasilkan secara serampangan melalui gabungan (dengan tempat-tempat sesekali untuk SQL Injection) di dalam kode yang tidak ada yang bisa mengerti?

Anda pikir Anda melakukan pekerjaan dengan baik, tetapi sekarang, pengembang baru yang bergabung dengan proyek harus menghadapi lebih banyak kerumitan daripada sebelumnya:

  • Cara lama memperlakukan permintaan,

  • Cara MVC,

  • Mekanisme penebangan yang lama,

  • Kerangka kerja logging,

  • Akses langsung ke database dengan query SQL dibangun dengan cepat,

  • ORM.

Pada satu proyek yang saya kerjakan, ada empat kerangka kerja logging (!) Yang digunakan berdampingan (ditambah logging manual). Alasannya adalah bahwa setiap kali seseorang ingin mencatat sesuatu tidak ada pendekatan umum untuk melakukannya, jadi alih-alih mempelajari kerangka kerja baru (yang dalam semua kasus hanya digunakan dalam 5% dari basis kode), seseorang hanya akan menambahkan yang lain dia sudah tahu. Bayangkan kekacauan itu.

Pendekatan yang lebih baik adalah dengan memperbaiki basis kode satu langkah pada satu waktu. Mengambil sekali lagi contoh penebangan, refactoring akan terdiri dari langkah-langkah kecil berikut:

  • Temukan semua tempat di mana legacy logging dilakukan (yaitu ketika file log diakses secara langsung) dan pastikan semuanya memanggil metode yang sama.

  • Pindahkan kode ini ke perpustakaan khusus, jika ada. Saya tidak ingin masuk log logika penyimpanan di kelas keranjang belanja saya.

  • Ubah, jika perlu, antarmuka metode pencatatan. Misalnya, kita dapat menambahkan level yang menunjukkan apakah pesan itu informal, atau peringatan atau kesalahan.

  • Gunakan metode yang baru di-refactored di fitur baru.

  • Bermigrasi ke kerangka kerja pencatatan: satu-satunya kode yang terpengaruh adalah kode di dalam perpustakaan khusus.

Arseni Mourzenko
sumber
1
Jawaban bagus sampai paragraf terakhir. Apakah Anda bermaksud atau tidak, Anda menyiratkan bahwa praktik yang baik tidak boleh digunakan untuk kode baru. -1
RubberDuck
2
@RubberDuck: ini bukan tentang praktik yang baik, ini tentang menulis kode yang secara radikal berbeda dari basis kode yang tersisa. Saya telah menjadi pengembang itu dan saya telah melihat konsekuensi dari apa yang telah saya lakukan: memiliki kode yang lebih baik secara drastis di antara kode yang buruk hanya memperburuk keadaan; apa yang membuatnya lebih baik adalah meningkatkan basis kode melalui langkah-langkah refactoring kecil. Saya akan menambahkan contoh dalam jawaban saya dalam beberapa menit.
Arseni Mourzenko
Saya akan DV lagi jika saya bisa. Hasil edit membuatnya lebih buruk. Memiliki empat metode berbeda untuk melakukan sesuatu dengan cara baru itu buruk, tetapi kesalahan orang yang menambahkan kerangka logging kedua. Tidak buruk dalam dirinya sendiri untuk menggambar garis di pasir dan memiliki kode bersih di samping kode busuk.
RubberDuck
@RubberDuck: Ini bukan tentang menambahkan kerangka kerja logging kedua , tetapi yang pertama. Orang yang menambahkan kerangka logging kedua melakukan itu hanya karena yang pertama digunakan hanya pada satu fitur kecil; jika basis kode direaktor seperti yang saya sarankan, ini tidak akan terjadi.
Arseni Mourzenko
Saya pikir Anda dan saya setuju, tetapi jawaban Anda berbunyi sedemikian rupa sehingga tidak mendorong perbaikan. Aku hanya tidak bisa bergabung dengan itu.
RubberDuck
3

Jika Anda melakukan tinjauan kode sebagai bagian dari proses pengembangan Anda; maka Anda harus menetapkan aturan yang Anda 'menilai' kode yang Anda tinjau.

Ini harus masuk ke 'definisi selesai' Anda dan bisa menjadi panduan gaya, dokumen arsitektur untuk basis kode atau analisis statis, memeriksa persyaratan hukum, apa pun perusahaan memutuskan adalah persyaratan untuk 'kualitas kode'

Setelah Anda menerapkan ulasan kode tempat ini menjadi hal yang biasa, apakah panduan gaya telah diikuti, apakah kami memiliki persentase cakupan tes yang diperlukan, dll.

Jika Anda tidak memiliki ini di tempat maka ulasan kode hanya bisa menjadi pertempuran siapa yang memiliki kemauan pengkodean terbesar atau, seperti dalam situasi Anda, mempertanyakan panggilan penilaian tentang refactoring vs fitur yang dilakukan sebelum batas waktu. Yang hanya membuang waktu semua orang.

Ewan
sumber
3

Masalah utama Anda adalah bahwa tinjauan kode pada fitur baru yang signifikan adalah waktu yang salah untuk melakukan diskusi ini. Pada saat itu sudah terlambat untuk melakukan perubahan kecil. Tempat yang tepat adalah dalam tahap perencanaan, atau paling lambat dalam tinjauan desain awal. Jika perusahaan Anda tidak melakukan tinjauan awal tersebut setidaknya secara informal, Anda harus berusaha mengubah budaya itu terlebih dahulu.

Langkah selanjutnya adalah membuat diri Anda diundang ke pertemuan-pertemuan itu, dan memiliki ide-ide produktif di pertemuan-pertemuan itu. Sebagian besar itu berarti tidak mencoba mengubah segalanya dalam semalam, tetapi mencari potongan seukuran gigitan yang dapat Anda isolasi dan atasi. Potongan-potongan itu akan bertambah secara signifikan dari waktu ke waktu.

Dengan kata lain, kuncinya adalah untuk secara teratur menyarankan perubahan yang lebih kecil menjelang dimulainya proyek, dan bukannya ditolak menyarankan perubahan yang lebih besar menjelang akhir proyek.

Karl Bielefeldt
sumber
2

Di tempat-tempat di mana saya telah melakukan review kode, saya akan menerima dan menyetujui pengiriman kode, jika disertai dengan komitmen untuk melakukan (setidaknya) beberapa refactoring. Baik itu bug yang diajukan, sebagai cerita, atau janji untuk mengirim ulasan lain dengan (sebagian) refactoring dilakukan.

Dalam kasus ini, jika saya adalah orang yang menulis kode untuk ditinjau, saya biasanya menyiapkan dua perubahan, satu dengan fitur baru atau perbaikan bug, lalu yang lain DAN beberapa pembersihan. Dengan begitu, perubahan pembersihan tidak mengurangi fitur baru atau perbaikan bug, tetapi mudah disebut sebagai tanda "ya, saya tahu ini tidak memperbaiki masalah ini, tetapi di sana ada" bersih-bersih "itu tidak".

Vatine
sumber