Rekonsiliasi Boy Scout Rule dan Opportunistic Refactoring dengan ulasan kode

Saya sangat percaya pada Aturan Pramuka :

Selalu periksa modul dalam pembersih daripada ketika Anda memeriksanya. "Tidak peduli siapa penulis aslinya, bagaimana jika kita selalu berusaha, tidak peduli seberapa kecil, untuk meningkatkan modul. Apa hasilnya? Saya pikir jika kita semua mengikuti aturan sederhana itu, kita akan melihat akhir dari kemunduran tanpa henti dari sistem perangkat lunak kita, sebagai gantinya, sistem kita akan secara bertahap menjadi lebih baik dan lebih baik ketika mereka berevolusi. daripada hanya individu yang merawat bagian kecil mereka sendiri.

Saya juga sangat percaya pada ide terkait Refactoring Opportunistik :

Meskipun ada tempat untuk beberapa upaya refactoring terjadwal, saya lebih memilih untuk mendorong refactoring sebagai kegiatan oportunistik, dilakukan kapan saja dan di mana pun kode perlu dibersihkan - oleh siapa pun. Artinya adalah bahwa setiap saat seseorang melihat beberapa kode yang tidak sejelas seharusnya, mereka harus mengambil kesempatan untuk memperbaikinya di sana dan kemudian - atau setidaknya dalam beberapa menit.

Khususnya perhatikan kutipan berikut dari artikel refactoring:

Saya waspada terhadap praktik pengembangan yang menyebabkan gesekan untuk refactoring oportunistik ... Perasaan saya adalah bahwa sebagian besar tim tidak melakukan cukup refactoring, jadi penting untuk memperhatikan apa pun yang menghambat orang untuk melakukannya. Untuk membantu menghilangkan ini, perhatikan kapan saja Anda merasa kecil hati untuk melakukan refactoring kecil, yang Anda yakin hanya akan memakan waktu satu atau dua menit. Setiap penghalang seperti itu adalah bau yang harus memicu percakapan. Jadi buatlah catatan tentang keputusasaan dan bawa bersama tim. Paling tidak itu harus dibahas selama retrospektif Anda berikutnya.

Tempat saya bekerja, ada satu praktek pembangunan yang menyebabkan berat gesekan - Kode Ulasan (CR). Setiap kali saya mengubah apa pun yang tidak dalam lingkup "tugas" saya, saya ditegur oleh pengulas saya bahwa saya membuat perubahan lebih sulit untuk ditinjau. Ini terutama benar ketika refactoring terlibat, karena itu membuat perbandingan diff "baris demi baris" menjadi sulit. Pendekatan ini adalah standar di sini, yang berarti refactoring oportunistik jarang dilakukan, dan hanya refactoring "terencana" (yang biasanya terlalu sedikit, terlalu terlambat) terjadi, jika sama sekali.

Saya mengklaim bahwa manfaatnya sepadan, dan bahwa 3 pengulas akan bekerja sedikit lebih keras (untuk benar-benar memahami kode sebelum dan sesudah, daripada melihat pada ruang lingkup sempit di mana garis berubah - ulasan itu sendiri akan lebih baik karena itu saja ) sehingga 100 pengembang berikutnya yang membaca dan memelihara kode akan mendapat manfaat. Ketika saya mempresentasikan argumen ini, pengulas saya, mereka mengatakan mereka tidak memiliki masalah dengan refactoring saya, asalkan tidak dalam CR yang sama. Namun saya mengklaim ini adalah mitos:

(1) Sebagian besar waktu Anda hanya menyadari apa dan bagaimana Anda ingin refactor ketika Anda berada di tengah-tengah tugas Anda. Seperti Martin Fowler katakan:

Ketika Anda menambahkan fungsionalitas, Anda menyadari bahwa beberapa kode yang Anda tambahkan berisi beberapa duplikasi dengan beberapa kode yang ada, jadi Anda perlu refactor kode yang ada untuk membersihkan sesuatu ... Anda mungkin mendapatkan sesuatu yang berfungsi, tetapi menyadari bahwa itu akan menjadi lebih baik jika interaksi dengan kelas yang ada diubah. Ambil kesempatan itu untuk melakukan itu sebelum Anda menganggap diri Anda sudah selesai.

(2) Tidak ada yang akan memandang positif Anda melepaskan "refactoring" CR yang seharusnya tidak Anda lakukan. CR memiliki overhead tertentu dan manajer Anda tidak ingin Anda "membuang waktu" untuk refactoring. Ketika dibundel dengan perubahan yang seharusnya Anda lakukan, masalah ini diminimalkan.

Masalah ini diperburuk oleh Resharper, karena setiap file baru yang saya tambahkan ke perubahan (dan saya tidak bisa tahu sebelumnya file mana yang akhirnya akan berubah) biasanya dipenuhi dengan kesalahan dan saran - sebagian besar tepat dan benar-benar layak pemasangan.

Hasil akhirnya adalah saya melihat kode yang mengerikan, dan saya membiarkannya di sana. Ironisnya, saya merasa bahwa memperbaiki kode seperti itu tidak hanya tidak akan memperbaiki posisi saya, tetapi sebenarnya menurunkannya dan melukis saya sebagai orang yang "tidak fokus" yang membuang waktu memperbaiki hal-hal yang tidak dipedulikan orang lain daripada melakukan pekerjaannya. Saya merasa tidak enak karena saya benar-benar membenci kode buruk dan tidak tahan melihatnya, apalagi menyebutnya dari metode saya!

Adakah pemikiran tentang bagaimana saya dapat memperbaiki situasi ini?

OK, jadi sekarang ada lebih banyak hal di sini daripada yang cocok untuk komentar.

tl; dr

Intuisi Anda tentang apa yang harus Anda lakukan (refactoring saat Anda pergi) benar.

Kesulitan Anda menerapkan ini - mengingat bahwa Anda harus bekerja di sekitar sistem ulasan kode yang buruk - datang ke kesulitan Anda memanipulasi kode sumber dan VCS Anda. Banyak orang mengatakan bahwa Anda dapat dan harus membagi perubahan Anda (ya, bahkan di dalam file) menjadi beberapa komit, tetapi Anda tampaknya sulit memercayai hal ini.

Anda benar-benar dapat melakukan ini. Itu benar-benar adalah apa yang kita menyarankan. Anda benar-benar harus belajar untuk memanfaatkan alat pengeditan, manipulasi sumber, dan kontrol versi Anda secara maksimal. Jika Anda menginvestasikan waktu dalam mempelajari cara menggunakannya dengan baik, itu membuat hidup jauh lebih mudah.

Masalah alur kerja / politik kantor

Saya pada dasarnya akan membuat rekomendasi yang sama dengan GlenH7 yang Anda buat dua komit - satu dengan hanya refactorings dan (terbukti atau jelas) tidak ada perubahan fungsional, dan satu dengan perubahan fungsional yang bersangkutan.

Mungkin berguna, jika Anda menemukan banyak kesalahan, untuk memilih satu kategori kesalahan untuk diperbaiki dalam CR tunggal. Kemudian Anda memiliki satu komit dengan komentar seperti "kode dedupe", "memperbaiki kesalahan tipe-X", atau apa pun. Karena ini membuat satu jenis perubahan, mungkin di banyak tempat, itu harus sepele untuk ditinjau. Itu berarti Anda tidak dapat memperbaiki setiap kesalahan yang Anda temukan, tetapi mungkin membuat penyelundupan tidak begitu menyakitkan.

Masalah VCS

Memisahkan perubahan yang dibuat ke sumber kerja Anda menjadi beberapa komit seharusnya tidak menjadi tantangan. Anda belum mengatakan apa yang Anda gunakan, tetapi alur kerja yang mungkin adalah:

1. jika Anda menggunakan git, Anda memiliki alat yang sangat bagus untuk ini

   • Anda dapat menggunakan git add -iuntuk pementasan interaktif dari baris perintah
   • Anda dapat menggunakan git guidan memilih bakhil dan baris individu ke panggung (ini adalah salah satu dari beberapa alat GUI terkait VCS yang sebenarnya saya lebih suka ke baris perintah, yang lainnya menjadi editor gabungan 3-cara yang bagus)
   • Anda dapat membuat banyak komitmen kecil (perubahan individu, atau memperbaiki kelas bug yang sama di beberapa tempat) dan kemudian menyusun ulang, menggabungkannya atau membaginya dengan rebase -i

2. jika Anda tidak menggunakan git, VCS Anda mungkin masih memiliki alat untuk hal semacam ini, tapi saya tidak bisa memberi saran tanpa mengetahui sistem apa yang Anda gunakan

   Anda menyebutkan Anda menggunakan TFS - yang saya percaya kompatibel dengan git sejak TFS2013. Mungkin patut bereksperimen dengan menggunakan klon git lokal repo untuk bekerja. Jika ini dinonaktifkan atau tidak bekerja untuk Anda, Anda masih dapat mengimpor sumber ke repo git lokal, bekerja di sana, dan menggunakannya untuk ekspor komitmen akhir Anda.

3. Anda dapat melakukannya secara manual di VCS apa pun jika Anda memiliki akses ke alat dasar seperti diffdan patch. Ini lebih menyakitkan, tetapi tentu saja mungkin. Alur kerja dasar adalah:

   1. lakukan semua perubahan, tes, dll.
   2. gunakan diffuntuk membuat file tambalan (konteks atau terpadu) dengan semua perubahan sejak komit terakhir
   3. partisi itu menjadi dua file: Anda akan berakhir dengan satu file tambalan yang berisi refactorings, dan satu dengan perubahan fungsional
      • ini tidak sepenuhnya sepele - alat seperti emacs diff mode dapat membantu
   4. mendukung semuanya
   5. kembali ke komit terakhir, gunakan patchuntuk memutar ulang salah satu file tambalan, komit perubahan itu
      • NB. jika tambalan tidak berlaku bersih, Anda mungkin perlu memperbaiki rongsokan yang gagal secara manual
   6. ulangi 5 untuk file tambalan kedua

Sekarang Anda memiliki dua komitmen, dengan perubahan Anda dipartisi dengan tepat.

Catatan mungkin ada alat untuk membuat manajemen tambalan ini lebih mudah - Saya hanya tidak menggunakannya karena git melakukannya untuk saya.

Saya akan berasumsi bahwa Permintaan Perubahan besar dan formal di perusahaan Anda. Jika tidak, buat saja perubahan (jika memungkinkan) menjadi banyak komit kecil (seperti yang seharusnya).

Adakah pemikiran tentang bagaimana saya dapat memperbaiki situasi ini?

Terus melakukan apa yang Anda lakukan?

Maksud saya, semua pemikiran dan kesimpulan Anda sepenuhnya benar. Anda harus memperbaiki hal-hal yang Anda lihat. Orang tidak cukup merencanakan refactoring. Dan manfaat bagi seluruh kelompok lebih penting daripada merepotkan beberapa orang.

Apa yang bisa membantu adalah menjadi kurang agresif. Ulasan kode tidak boleh agresif "Mengapa Anda melakukan itu?", Mereka harus kolaboratif "Hai teman-teman, ketika saya di sini saya memperbaiki semua hal ini!". Bekerja (dengan pimpinan / manajer Anda jika mungkin) untuk mengubah budaya itu sulit, tetapi sangat penting untuk membuat tim pengembangan yang berfungsi tinggi.

Anda juga dapat bekerja (dengan pimpinan / manajer Anda jika mungkin) untuk memajukan pentingnya ide-ide ini dengan kolega Anda. Buat itu menjadi Anda bertanya "mengapa kalian tidak peduli tentang kualitas?" bukannya mereka bertanya "mengapa kamu selalu melakukan hal-hal yang tidak berguna ini?".

Saya punya banyak simpati untuk situasi Anda, tetapi beberapa saran konkret. Jika tidak ada yang lain, mungkin saya akan meyakinkan Anda bahwa seburuk sebuah situasi, itu bisa lebih buruk. Itu bisa SELALU lebih buruk. :-)

Pertama, saya pikir Anda memiliki (setidaknya) dua masalah dengan budaya Anda, bukan hanya satu. Satu masalah adalah pendekatan untuk refactoring, tetapi ulasan kode tampak seperti masalah yang berbeda. Saya akan mencoba untuk memisahkan pikiran saya.

Ulasan Kode

Saya berada di grup yang membenci ulasan kode. Grup ini dibentuk dengan menggabungkan dua grup dari bagian terpisah dari perusahaan. Saya berasal dari grup yang telah melakukan tinjauan kode selama beberapa tahun, dengan hasil yang umumnya baik. Sebagian besar dari kita percaya ulasan kode adalah penggunaan waktu kita yang baik. Kami bergabung menjadi grup yang lebih besar, dan sedekat yang kami tahu, bahwa grup "lain" bahkan belum pernah mendengar ulasan kode. Sekarang kami semua bekerja pada basis kode "mereka".

Hal-hal yang tidak baik ketika kami bergabung. Fitur-fitur baru terlambat 6-12 bulan, tahun demi tahun. Tumpukan bug sangat besar, tumbuh, dan melelahkan. Kepemilikan kode sangat kuat, terutama di antara "guru" paling senior. "Cabang fitur" kadang-kadang berlangsung bertahun-tahun dan membentang beberapa rilis; terkadang NOBODY tetapi satu pengembang melihat kode sebelum mencapai cabang utama. Sebenarnya "cabang fitur" menyesatkan, karena menunjukkan kode berada di suatu tempat di repositori. Lebih sering hanya pada sistem individu pengembang.

Manajemen setuju bahwa kami perlu "melakukan sesuatu" sebelum kualitas menjadi sangat rendah. :-) Jawaban mereka adalah Ulasan Kode. Ulasan Kode menjadi item resmi "Engkau", untuk mengawali setiap check-in. Alat yang kami gunakan adalah Papan Review.

Bagaimana cara kerjanya dalam budaya yang saya jelaskan? Lebih baik daripada tidak sama sekali, tetapi itu menyakitkan dan membutuhkan lebih dari satu tahun untuk mencapai tingkat kepatuhan minimal. Beberapa hal yang kami amati:

1. Alat yang Anda gunakan cenderung untuk memfokuskan ulasan kode dengan cara tertentu. Ini bisa jadi masalah. Review Board memberi Anda perbedaan garis-demi-garis yang bagus dan penuh warna dan memungkinkan Anda melampirkan komentar pada satu baris. Ini membuat sebagian besar pengembang mengabaikan semua baris yang tidak berubah. Itu bagus untuk perubahan kecil dan terisolasi. Ini tidak begitu baik untuk perubahan besar, potongan besar kode baru, atau kode yang memiliki 500 contoh fungsi berganti nama yang dicampur dengan 10 baris fungsi baru.
2. Meskipun kami berada di basis kode lama dan sakit yang sebagian besar belum pernah ditinjau sebelumnya, menjadi tidak sopan bagi pengulas untuk mengomentari apa pun yang BUKAN garis perubahan, bahkan untuk menunjukkan bug yang jelas. "Jangan ganggu saya, ini adalah check-in yang penting dan saya tidak punya waktu untuk memperbaiki bug."
3. Berbelanja untuk pengulas "mudah". Beberapa orang akan melihat tinjauan 10 file dengan 2000 baris yang diubah selama 3 menit dan klik "Kirim!". Semua orang dengan cepat mengetahui siapa orang-orang itu. Jika Anda benar-benar tidak ingin kode Anda ditinjau sejak awal, kirimkan ke resensi "mudah". Check-in Anda tidak akan melambat. Anda dapat membalas budi dengan menjadi peninjau "mudah" untuk kode-nya.
4. Jika Anda membenci ulasan kode, abaikan saja email yang Anda dapatkan dari Review Board. Abaikan tindak lanjut dari anggota tim Anda. Selama berminggu-minggu. Sampai Anda memiliki 3 lusin ulasan terbuka di antrian Anda dan nama Anda muncul di pertemuan kelompok beberapa kali. Kemudian jadikan resensi "mudah" dan kosongkan semua ulasan Anda sebelum jam makan siang.
5. Hindari mengirim kode Anda ke resensi "keras". (Jenis pengembang yang akan bertanya atau menjawab pertanyaan seperti ini.) Setiap orang dengan cepat mengetahui siapa pengulas "keras", sama seperti mereka mempelajari yang mudah. Jika ulasan kode adalah buang-buang waktu (™), maka membaca umpan balik terperinci tentang kode yang Anda miliki adalah buang waktu (™) dan merupakan penghinaan.
6. Ketika ulasan kode menyakitkan, orang menunda dan terus menulis lebih banyak kode. Anda mendapatkan lebih sedikit ulasan kode, tetapi masing-masing BESAR. Anda membutuhkan lebih banyak, tinjauan kode yang lebih kecil, yang berarti tim perlu memikirkan cara membuatnya semudah mungkin.

Saya pikir saya akan menulis beberapa paragraf tentang Ulasan Kode, tetapi ternyata saya kebanyakan menulis tentang budaya. Saya kira itu bermuara pada ini: review kode bisa baik untuk proyek, tetapi tim hanya mendapatkan manfaat yang layak mereka dapatkan.

Refactoring

Apakah grup saya membenci refactoring bahkan lebih daripada membenci ulasan kode? Tentu saja! Untuk semua alasan yang jelas yang telah disebutkan. Anda membuang-buang waktu saya dengan ulasan kode icky, dan Anda bahkan tidak menambahkan fitur atau memperbaiki bug!

Tetapi kode itu masih sangat membutuhkan refactoring. Bagaimana untuk melanjutkan?

1. Jangan sekali-kali mencampur perubahan refactoring dengan perubahan fungsional. Sejumlah orang telah menyebutkan poin ini. Jika ulasan kode adalah titik friksi, jangan tambahkan friksi. Ini lebih banyak pekerjaan, tetapi Anda harus merencanakan review terpisah dan check-in terpisah.
2. Mulai dari yang kecil. Sangat kecil. Bahkan lebih kecil dari itu. Gunakan refactor yang sangat kecil untuk secara bertahap mengajarkan orang bahwa refactoring dan review kode bukanlah kejahatan murni. Jika Anda dapat menyelinap dalam satu refactoring kecil per minggu tanpa terlalu banyak rasa sakit, dalam beberapa bulan Anda mungkin bisa lolos dengan dua per minggu. Dan mereka bisa sedikit lebih besar. Lebih baik daripada tidak.
3. Kami pada dasarnya TIDAK memiliki unit test. Jadi refactoring dilarang, kan? Belum tentu. Untuk beberapa perubahan, kompiler adalah unit test Anda. Fokus pada refactoring yang dapat Anda uji. Hindari perubahan jika tidak dapat diuji. Mungkin menghabiskan waktu menulis unit test sebagai gantinya.
4. Beberapa pengembang takut refactoring karena mereka takut SEMUA perubahan kode. Butuh waktu lama untuk mengenali tipe ini. Tulislah sepotong kode, biarlah itu sampai "bekerja", dan kemudian TIDAK PERNAH ingin mengubahnya. Mereka tidak selalu mengerti MENGAPA ini bekerja. Perubahan itu berisiko. Mereka tidak percaya diri untuk melakukan perubahan APAPUN, dan mereka pasti tidak akan mempercayai Anda. Refactoring seharusnya tentang perubahan kecil dan aman yang tidak mengubah perilaku. Ada pengembang yang gagasan "perubahan yang tidak mengubah perilaku" tidak dapat dibayangkan . Saya tidak tahu harus menyarankan apa dalam kasus ini. Saya pikir Anda harus mencoba bekerja di bidang yang tidak mereka pedulikan. Saya terkejut ketika saya mengetahui bahwa jenis ini bisa lama,

Mengapa Anda tidak melakukan keduanya, tetapi dengan komitmen terpisah?

Rekan-rekan Anda ada benarnya. Peninjauan kode harus mengevaluasi kode yang diubah oleh orang lain. Anda tidak boleh menyentuh kode yang Anda ulas untuk orang lain karena itu bias peran Anda sebagai peninjau.

Tetapi jika Anda melihat sejumlah masalah mencolok, ada dua opsi yang dapat Anda ikuti.

1. Jika ulasan kode dinyatakan baik-baik saja, ijinkan bagian yang Anda ulas untuk dilakukan dan kemudian refactor kode di bawah check-in kedua.

2. Jika tinjauan kode memiliki masalah yang perlu diperbaiki, minta pengembang untuk memperbaiki berdasarkan rekomendasi Resharper.

Saya pribadi benci ide ulasan kode pos komit. Peninjauan kode harus terjadi, saat Anda membuat perubahan kode. Apa yang saya bicarakan di sini adalah pemrograman pasangan. Saat Anda berpasangan, Anda mendapatkan ulasan secara gratis dan Anda mendapatkan ulasan kode yang lebih baik. Sekarang, saya mengungkapkan pendapat saya di sini, saya tahu orang lain berbagi ini, mungkin ada studi yang membuktikan ini.

Jika Anda bisa membuat peninjau kode Anda untuk dipasangkan dengan Anda, elemen agresif dari tinjauan kode harus menguap. Ketika Anda mulai membuat perubahan yang tidak dipahami, pertanyaan dapat diajukan pada titik perubahan, dibahas dan alternatif dieksplorasi yang dapat mengarah pada refactoring yang lebih baik. Anda akan mendapatkan tinjauan kode kualitas yang lebih tinggi karena pasangan akan dapat memahami cakupan perubahan yang lebih luas, dan tidak terlalu fokus pada perubahan baris demi baris yang Anda dapatkan dengan membandingkan kode berdampingan.

Tentu saja ini bukan jawaban langsung untuk masalah refactoring berada di luar lingkup perubahan yang sedang dikerjakan, tetapi saya berharap rekan-rekan Anda akan lebih memahami alasan di balik perubahan jika mereka ada di mana ketika Anda melakukan perubahan.

Selain itu, dengan asumsi Anda sedang melakukan TDD atau semacam refactor merah hijau, salah satu cara untuk memastikan bahwa Anda mendapatkan keterlibatan dari rekan Anda adalah dengan menggunakan teknik pemasangan pingpong. Cukup dijelaskan ketika driver diputar untuk setiap langkah dari siklus RGR, yaitu pasangan 1 menulis tes gagal, pasangan 2 memperbaikinya, pasangan 1 refaktor, pasangan 2 menulis tes gagal .... dan seterusnya.

Mungkin masalah ini mencerminkan masalah yang jauh lebih besar dengan budaya organisasi. Orang-orang tampaknya lebih tertarik untuk melakukan "pekerjaannya" dengan benar daripada membuat produk lebih baik, mungkin perusahaan ini memiliki budaya "menyalahkan" daripada budaya kolaboratif dan orang-orang tampaknya lebih tertarik untuk menutup sendiri daripada memiliki seluruh produk / visi perusahaan .

Menurut pendapat saya, Anda sepenuhnya benar, orang-orang yang meninjau kode Anda sepenuhnya salah jika mereka mengeluh karena Anda "menyentuh" ​​hal-hal di luar "tugas Anda", cobalah untuk meyakinkan orang-orang itu tetapi jangan pernah melanggar prinsip Anda, bagi saya ini adalah kualitas paling penting dari seorang profesional sejati.

Dan jika melakukan hal yang benar memberi Anda angka buruk dalam beberapa cara korporasi bodoh untuk mengevaluasi pekerjaan Anda, apa masalahnya ?, siapa yang ingin "memenangkan" permainan evaluasi ini di perusahaan yang gila ?, cobalah mengubahnya !, dan jika tidak mungkin atau Anda lelah, menemukan tempat lain, tetapi tidak pernah, tidak pernah melanggar prinsip Anda, adalah yang terbaik yang Anda miliki.

Terkadang, refactoring adalah hal yang buruk. Namun, bukan karena alasan yang diberikan oleh pengulas kode Anda; sepertinya mereka tidak terlalu peduli dengan kualitas kodenya, dan Anda memang peduli, itu mengagumkan. Tetapi ada dua alasan yang harus menghentikan Anda dari refactoring : 1. Anda tidak dapat menguji perubahan yang Anda buat dengan tes otomatis (tes unit atau apa pun) atau 2. Anda membuat perubahan pada beberapa kode yang Anda tidak mengerti baik; yaitu, Anda tidak memiliki pengetahuan khusus domain untuk mengetahui perubahan apa yang harus Anda lakukan.

1. Jangan refactor saat Anda tidak dapat menguji perubahan yang Anda buat dengan tes otomatis.

Orang yang melakukan tinjauan kode Anda harus dapat memastikan bahwa perubahan yang Anda buat tidak merusak apa pun yang dulu berfungsi. Ini adalah alasan terbesar untuk meninggalkan kode kerja. Ya, pasti akan lebih baik untuk memperbaiki fungsi 1000 baris (yang benar-benar kekejian!) Menjadi banyak fungsi, tetapi jika Anda tidak dapat mengotomatiskan tes Anda maka bisa sangat sulit untuk meyakinkan orang lain bahwa Anda melakukan segalanya Baik. Saya pasti telah membuat kesalahan ini sebelumnya.

Sebelum refactoring, pastikan ada unit test. Jika tidak ada tes unit, tulis beberapa! Kemudian refactor pergi, dan pengulas kode Anda tidak akan memiliki (sah) alasan untuk kesal.

Jangan refactor potongan kode yang membutuhkan pengetahuan khusus Domain yang tidak Anda miliki.

Tempat saya bekerja memiliki banyak kode teknik-berat kimia. Basis kode menggunakan idiom yang umum untuk insinyur kimia. Jangan pernah membuat perubahan yang idiomatis ke suatu bidang. Ini mungkin tampak seperti ide bagus untuk mengubah nama variabel yang disebut xuntuk molarConcentration, tapi coba tebak? Dalam semua teks kimia, konsentrasi molar diwakili dengan a x. Mengganti namanya membuatnya lebih sulit untuk mengetahui apa yang sebenarnya terjadi dalam kode untuk orang-orang di bidang itu.

Alih-alih mengganti nama variabel idiomatik, cukup beri komentar yang menjelaskan apa itu variabel. Jika tidak idiomatis, silakan ganti namanya. Jangan meninggalkan i, j, k, x, y, dll mengambang sekitar ketika nama-nama yang lebih baik akan bekerja.

Aturan praktis untuk singkatan: jika, ketika orang berbicara, mereka menggunakan singkatan, tidak apa-apa untuk menggunakan singkatan itu dalam kode. Contoh dari basis kode di tempat kerja:

Kami memiliki konsep berikut yang selalu disingkat ketika membicarakannya: "Area Kepedulian" menjadi "AOC", "Ledakan awan uap" menjadi VCE, hal-hal seperti itu. Dalam basis kode kami, seseorang mere-refactored semua instance yang disebut aoc ke AreaOfConcern, VCE ke vaporCloudExplosion, nPlanes ke confiningPlanesCount ... yang membuat kode itu sebenarnya jauh lebih sulit dibaca untuk orang yang memiliki pengetahuan khusus domain. Saya juga bersalah melakukan hal-hal seperti ini.

Ini mungkin sebenarnya tidak berlaku dalam situasi Anda sama sekali, tetapi ada pemikiran saya tentang masalah ini.

Pertanyaan ini sekarang memiliki dua masalah berbeda - kemudahan meninjau perubahan dalam tinjauan kode, dan waktu yang dihabiskan untuk refactoring.

Di mana saya bekerja, ada satu praktik pengembangan yang menyebabkan gesekan besar - Code Review (CR). Setiap kali saya mengubah apa pun yang tidak dalam lingkup "tugas" saya, saya ditegur oleh pengulas saya bahwa saya membuat perubahan lebih sulit untuk ditinjau.

Seperti jawaban lain katakan - dapatkah Anda memisahkan checkin refactoring dari checkin perubahan kode (tidak harus sebagai ulasan terpisah)? Bergantung pada alat yang Anda gunakan untuk melakukan review kode, Anda harus dapat melihat perbedaan antara revisi tertentu saja (Crucible Atlassian pasti melakukan ini).

(2) Tidak ada yang akan memandang positif Anda melepaskan CR "refactoring" yang seharusnya tidak Anda lakukan. CR memiliki overhead tertentu dan manajer Anda tidak ingin Anda "membuang waktu" untuk refactoring. Ketika dibundel dengan perubahan yang seharusnya Anda lakukan, masalah ini diminimalkan.

Jika refactoring mudah dan membuat kode lebih mudah dipahami (yang harus Anda lakukan jika hanya refactoring) maka review kode tidak perlu waktu lama untuk diselesaikan dan harus meminimalkan biaya overhead yang dimenangkan dalam sekop ketika seseorang harus datang dan melihat kode lagi di masa depan. Jika bos Anda tidak dapat melakukan hal ini, maka Anda mungkin perlu dengan lembut mendorong mereka ke beberapa sumber yang membahas mengapa Peraturan Pramuka mengarah ke hubungan yang lebih produktif dengan kode Anda. Jika Anda dapat meyakinkan mereka bahwa "buang-buang waktu" sekarang akan menghemat dua, lima atau sepuluh kali lebih banyak nanti ketika Anda / orang lain kembali untuk melakukan lebih banyak pekerjaan pada kode itu maka masalah Anda akan hilang.

Masalah ini diperburuk oleh Resharper, karena setiap file baru yang saya tambahkan ke perubahan (dan saya tidak bisa tahu sebelumnya file mana yang akhirnya akan berubah) biasanya dipenuhi dengan kesalahan dan saran - sebagian besar tepat dan benar-benar layak pemasangan.

Sudahkah Anda mencoba membawa masalah ini menjadi perhatian kolega Anda dan mendiskusikan mengapa hal itu layak diperbaiki? Dan dapatkah salah satu dari mereka diperbaiki secara otomatis (dengan asumsi Anda memiliki cakupan pengujian yang cukup untuk mengonfirmasi bahwa Anda belum merusak barang-barang dengan refactoring otomatis)? Kadang-kadang itu tidak "sepadan dengan waktu Anda" untuk melakukan refactoring jika itu benar-benar hal yang rewel. Apakah seluruh tim Anda menggunakan ReSharper? Jika ya, apakah Anda memiliki kebijakan bersama tentang peringatan / peraturan apa yang ditegakkan? Jika tidak, sekali lagi mungkin Anda harus menunjukkan di mana alat ini membantu Anda mengidentifikasi area kode yang mungkin menjadi sumber rasa sakit di masa depan.

Saya dapat mengingat 25 tahun yang lalu kode "pembersihan" yang kebetulan saya kerjakan untuk tujuan lain, sebagian besar dengan memformat ulang blok komentar dan penjajaran tab / kolom untuk membuat kode rapi dan karenanya lebih mudah dipahami (tidak ada perubahan fungsional aktual) . Saya kebetulan suka kode yang rapi dan tertata dengan baik. Yah, pengembang senior sangat marah. Ternyata mereka menggunakan semacam perbandingan file (diff) untuk melihat apa yang telah berubah, dibandingkan dengan salinan pribadi file tersebut, dan sekarang itu memberikan segala macam false positive. Saya harus menyebutkan bahwa perpustakaan kode kami ada di mainframe dan tidak memiliki kontrol versi - Anda pada dasarnya menimpa apa pun yang ada di sana, dan hanya itu.

Moral dari cerita ini? Saya tidak tahu. Saya kira terkadang Anda tidak bisa menyenangkan siapa pun selain diri Anda sendiri. Aku tidak membuang-buang waktu (di mata saya) - kode dibersihkan adalah lebih mudah untuk membaca dan memahami. Hanya saja alat kontrol perubahan primitif yang digunakan oleh orang lain memberikan beberapa pekerjaan ekstra untuk mereka. Alat-alat itu terlalu primitif untuk mengabaikan ruang / tab dan komentar yang direfleksikan.

Jika Anda dapat membagi perubahan yang diminta dan refactor yang tidak diminta menjadi (banyak) komitmen terpisah, seperti yang dicatat oleh @Useless, @Telastyn, dan lainnya, maka itulah yang terbaik yang dapat Anda lakukan. Anda masih akan mengerjakan CR tunggal, tanpa biaya administrasi untuk membuat "refactoring". Pertahankan komitmen Anda kecil dan fokus.

Alih-alih memberi Anda beberapa nasihat tentang cara melakukannya, saya lebih suka mengarahkan Anda ke penjelasan yang jauh lebih besar (pada kenyataannya, sebuah buku) dari seorang spesialis. Dengan cara ini, Anda akan dapat belajar lebih banyak. Baca Bekerja Efektif dengan Legacy Code (oleh Michael Feathers) . Buku ini dapat mengajarkan Anda bagaimana melakukan refactoring, diselingi dengan perubahan fungsional.

Topik yang dibahas meliputi:

• Memahami mekanisme perubahan perangkat lunak: menambahkan fitur, memperbaiki bug, meningkatkan desain, mengoptimalkan kinerja
• Mendapatkan kode lawas ke dalam harness uji
• Menulis tes yang melindungi Anda dari memperkenalkan masalah baru
• Teknik yang dapat digunakan dengan bahasa atau platform apa pun — dengan contoh dalam Java, C ++, C, dan C #
• Secara akurat mengidentifikasi di mana perubahan kode perlu dilakukan
• Mengatasi sistem warisan yang tidak berorientasi objek
• Menangani aplikasi yang tampaknya tidak memiliki struktur apa pun

Buku ini juga mencakup katalog dua puluh empat teknik pemecahan ketergantungan yang membantu Anda bekerja dengan elemen-elemen program secara terpisah dan membuat perubahan yang lebih aman.

Saya juga sangat percaya dengan Aturan Boyscout dan Opportunistic Refactoring. Masalahnya sering kali dalam mendapatkan pembelian manajemen. Refactoring datang dengan risiko dan biaya. Apa yang sering diabaikan oleh manajemen adalah begitu pula utang teknis.

Itu sifat manusia. Kita terbiasa berurusan dengan masalah nyata, bukan berusaha mencegahnya. Kami reaktif, bukan proaktif.

Perangkat lunak tidak berwujud dan jadi sulit untuk dipahami bahwa, seperti mobil, ia perlu diservis. Tidak ada orang yang beralasan yang akan membeli mobil dan tidak pernah menanganinya. Kami menerima bahwa kelalaian seperti itu akan mengurangi umur panjangnya dan, dalam jangka panjang, akan lebih mahal.

Terlepas dari kenyataan bahwa banyak manajer memahami hal ini, mereka bertanggung jawab atas perubahan kode produksi. Ada politik yang membuatnya lebih mudah untuk dibiarkan begitu saja. Seringkali orang yang perlu diyakinkan benar-benar di atas manajer Anda dan dia tidak ingin berjuang untuk mendapatkan "ide-ide hebat" Anda ke dalam produksi.

Sejujurnya, manajer Anda tidak selalu yakin bahwa obat Anda, pada kenyataannya, sama bagusnya dengan yang Anda pikirkan. (Minyak ular?) Ada keahlian menjual. Adalah tugas Anda untuk membantunya melihat manfaatnya.

Manajemen tidak membagikan prioritas Anda. Kenakan topi manajemen Anda dan lihat dengan mata manajemen. Anda akan lebih mungkin menemukan sudut yang tepat. Bersikaplah gigih. Anda akan membuat lebih banyak perubahan dengan tidak membiarkan respons negatif pertama menghalangi Anda.

Jika Anda dapat membagi perubahan yang diminta dan refactor yang tidak diminta menjadi dua permintaan perubahan yang terpisah, seperti dicatat oleh @ GlenH7, maka itu adalah yang terbaik yang dapat Anda lakukan. Anda kemudian bukan "orang yang membuang-buang waktu" tetapi "orang yang bekerja dua kali lebih keras".

Jika Anda sering menemukan diri Anda dalam situasi yang Anda tidak dapat membaginya, karena pekerjaan yang diminta sekarang memerlukan refactor yang tidak diminta untuk dikompilasi, saya akan menyarankan Anda bersikeras memiliki alat untuk memeriksa secara otomatis untuk standar pengkodean, menggunakan argumen yang diuraikan di sini (Resharper peringatan sendiri mungkin merupakan kandidat yang baik, saya tidak terbiasa dengannya). Argumen-argumen itu semuanya valid, tetapi ada satu tambahan yang dapat Anda gunakan untuk keuntungan Anda: memiliki tes-tes itu sekarang membuat tugas Anda untuk lulus tes pada setiap komit.

Jika perusahaan Anda tidak ingin "membuang waktu untuk refactoring" (pertanda buruk di pihak mereka), maka mereka harus memberikan kepada Anda file "peringatan penindasan" (setiap alat memiliki mekanisme sendiri) sehingga Anda tidak terganggu lagi dengan peringatan seperti itu. Saya mengatakan ini untuk memberi Anda opsi untuk berbagai skenario, bahkan yang terburuk. Lebih baik untuk memiliki perjanjian yang dinyatakan dengan jelas antara Anda dan perusahaan Anda, juga pada kualitas kode yang diharapkan, daripada asumsi implisit di setiap sisi.