Saya menyusun beberapa pedoman untuk ulasan kode. Kami belum memiliki satu proses formal dan berusaha untuk memformalkannya. Dan tim kami didistribusikan secara geografis.
Kami menggunakan TFS untuk kontrol sumber (kami juga menggunakannya untuk tugas / pelacakan bug / manajemen proyek, tetapi kami memindahkannya ke JIRA ) dengan Visual Studio 2008 untuk pengembangan.
Apa hal-hal yang Anda cari ketika melakukan tinjauan kode?
- Ini adalah hal-hal yang saya pikirkan
- Menegakkan aturan FxCop (kami adalah toko Microsoft)
- Periksa kinerja (alat apa pun?) Dan keamanan (berpikir tentang menggunakan OWASP - perayap kode ) dan keamanan utas
- Patuhi konvensi penamaan
- Kode harus mencakup kasus tepi dan kondisi batas
- Harus menangani pengecualian dengan benar (jangan menelan pengecualian)
- Periksa apakah fungsionalitas diduplikasi di tempat lain
- Badan metode harus berukuran kecil (20-30 baris), dan metode harus melakukan satu hal dan satu hal saja (tidak ada efek samping dan menghindari penggabungan temporal -)
- Jangan lulus / mengembalikan null dalam metode
- Hindari kode mati
- Dokumen metode / properti / variabel publik dan dilindungi
Hal lain apa yang harus kita perhatikan?
Saya mencoba untuk melihat apakah kita dapat mengukur proses peninjauan (itu akan menghasilkan output yang identik ketika ditinjau oleh orang yang berbeda) Contoh: Mengatakan "metode body seharusnya tidak lebih dari 20-30 baris kode" dibandingkan dengan mengatakan "metode tubuh harus kecil ".
Atau apakah tinjauan kode sangat subjektif (dan akan berbeda dari satu resensi ke yang lain)?
Tujuannya adalah untuk memiliki sistem penandaan (katakanlah -1 poin untuk setiap pelanggaran aturan FxCop, -2 poin karena tidak mengikuti konvensi penamaan, 2 poin untuk refactoring, dll.) Sehingga pengembang akan lebih berhati-hati ketika mereka memeriksa kode mereka. Dengan cara ini, kami dapat mengidentifikasi pengembang yang secara konsisten menulis kode baik / buruk. Tujuannya adalah meminta resensi menghabiskan waktu maksimal 30 menit, untuk melakukan tinjauan (saya tahu ini subjektif, mengingat fakta bahwa changeset / revisi mungkin menyertakan banyak file / perubahan besar pada arsitektur yang ada, dll., Tetapi Anda mendapatkan ide umum, peninjau tidak boleh menghabiskan hari meninjau kode seseorang).
Apa tujuan / sistem terukur lain yang Anda ikuti untuk mengidentifikasi kode baik / buruk yang ditulis oleh pengembang?
Referensi buku: Kode Bersih: Buku pegangan keahlian pengerjaan perangkat lunak tangkas oleh Robert Martin .
sumber
this.Foo().FooBar().FooBarBar();Jika objek yang dikembalikan ke sini dari Foo adalah null, Anda pasti dapat menghindari "Referensi objek yang tidak disetel ke instance objek" ketika memanggil FooBar ()Jawaban:
Menilai individu dalam ulasan bertentangan dengan sistem paling sukses yang pernah saya gunakan, mungkin semuanya. Tetapi tujuan yang saya coba capai selama lebih dari 20 tahun adalah lebih sedikit bug dan peningkatan produktivitas per-insinyur-jam. Jika menilai individu adalah tujuan, saya kira ulasan dapat digunakan. Saya belum pernah melihat situasi di mana diperlukan, sebagai pekerja atau sebagai pemimpin.
Beberapa studi objektif (Fagan, dll.) Dan banyak kearifan populer menunjukkan bahwa hubungan teman sebaya memfasilitasi ulasan kode yang ditujukan untuk mengurangi bug dan meningkatkan produktivitas. Manajer yang bekerja dapat berpartisipasi sebagai pekerja, tetapi bukan sebagai manajer. Poin diskusi dicatat, perubahan untuk memuaskan pengulas umumnya adalah hal yang baik tetapi tidak diperlukan. Karenanya hubungan teman sebaya.
Setiap alat otomatis yang dapat diterima tanpa analisis lebih lanjut atau penilaian baik - serat di C, C ++, Java. Kompilasi teratur. Compiler benar-benar bagus dalam menemukan bug kompiler. Mendokumentasikan penyimpangan dalam pemeriksaan otomatis terdengar seperti dakwaan halus pemeriksaan otomatis. Arahan kode (seperti halnya Java) yang memungkinkan penyimpangan cukup berbahaya, IMHO. Bagus untuk debugging, untuk memungkinkan Anda mendapatkan inti permasalahan, dengan cepat. Tidak begitu baik untuk ditemukan di 50.000 kode blok non-komentar yang Anda dokumentasikan dengan buruk.
Beberapa aturan bodoh tapi mudah ditegakkan; default untuk setiap pernyataan switch bahkan ketika mereka tidak terjangkau, misalnya. Maka itu hanya kotak centang, dan Anda tidak perlu menghabiskan waktu dan menguji uang dengan nilai-nilai yang tidak cocok dengan apa pun. Jika Anda memiliki aturan , Anda akan memiliki kebodohan , mereka terkait erat . Manfaat aturan apa pun harus sebanding dengan kebodohannya, dan hubungan itu harus diperiksa secara berkala.
Di sisi lain, "Ini berjalan" tidak ada kebajikan sebelum ditinjau, atau pertahanan dalam peninjauan. Jika pengembangan mengikuti model air terjun , Anda ingin melakukan peninjauan saat pengkodean selesai 85%, sebelum kesalahan rumit ditemukan dan dikerjakan, karena peninjauan adalah cara yang lebih murah untuk menemukannya. Karena kehidupan nyata bukanlah model air terjun, kapan mengulasnya agak merupakan seni dan merupakan norma sosial. Orang yang benar-benar akan membaca kode Anda dan mencari masalah di dalamnya adalah emas murni. Manajemen yang mendukung ini secara berkelanjutan adalah mutiara di atas harga. Ulasan harus seperti checkin- awal dan sering .
Saya menemukan hal-hal ini bermanfaat:
1) Tidak ada perang gaya . Ke mana perginya kurung kurawal buka hanya akan dikenakan pemeriksaan konsistensi dalam file yang diberikan. Semua sama. Baiklah kalau begitu. Kedalaman indentasi Ditto ** dan lebar tab ** . Sebagian besar organisasi menemukan bahwa mereka membutuhkan standar umum untuk tab, yang digunakan sebagai ruang besar.
2) `Ragged
teks yang tidak
untuk konten.`
BTW, K&R membuat indentasi lima (LIMA) ruang, sehingga banding ke otoritas tidak berharga. Bersikaplah konsisten.
3) Salinan file bernomor, tidak berubah, tersedia untuk umum untuk ditinjau harus ditunjuk selama 72 jam atau lebih sebelum peninjauan.
4) Tidak ada desain on the the fly. Jika ada masalah, atau ada masalah, perhatikan lokasinya, dan terus bergerak.
5) Pengujian yang melewati semua jalur dalam lingkungan pengembangan adalah ide yang sangat, sangat, sangat, baik. Pengujian yang membutuhkan data eksternal besar-besaran, sumber daya perangkat keras, penggunaan situs pelanggan, dll. Adalah pengujian yang menghabiskan banyak uang dan tidak akan menyeluruh.
6) Format file non- ASCII dapat diterima jika pembuatan, tampilan, edit, dll. Alat ada atau dibuat pada awal pengembangan. Ini adalah bias pribadi saya, tetapi di dunia di mana OS dominan tidak dapat keluar dengan caranya sendiri dengan kurang dari 1 gigabyte RAM, saya tidak dapat mengerti mengapa file kurang dari, katakanlah, 10 megabyte harus menjadi apa pun selain ASCII atau format lain yang didukung secara komersial. Ada standar untuk grafik, suara, film, dapat dieksekusi, dan alat yang menyertainya. Tidak ada alasan untuk file yang berisi representasi biner dari sejumlah objek.
Untuk pemeliharaan, refactoring, atau pengembangan kode yang dirilis, satu kelompok rekan kerja saya telah menggunakan ulasan oleh satu orang lain, duduk di layar dan melihat perbedaan lama dan baru , sebagai pintu gerbang ke cabang check-in. Saya menyukainya, murah, cepat, relatif mudah dilakukan. Jalan-jalan untuk orang-orang yang belum membaca kode di muka dapat mendidik untuk semua tetapi jarang memperbaiki kode pengembang.
Jika Anda terdistribusi secara geografis, melihat perbedaan di layar saat berbicara dengan orang lain yang melihat hal yang sama akan relatif mudah. Itu mencakup dua orang yang melihat perubahan. Untuk grup yang lebih besar yang telah membaca kode yang dimaksud, beberapa situs tidak jauh lebih sulit daripada semua dalam satu ruangan. Beberapa kamar dihubungkan oleh layar komputer bersama dan kotak jongkok bekerja sangat baik, IMHO. Semakin banyak situs, semakin banyak manajemen rapat diperlukan. Seorang manajer sebagai fasilitator dapat memperoleh penghasilan mereka di sini. Ingatlah untuk terus memberikan polling ke situs-situs yang tidak Anda kunjungi.
Pada satu titik, organisasi yang sama memiliki pengujian unit otomatis yang digunakan sebagai pengujian regresi. Itu sangat bagus. Tentu saja kami kemudian mengubah platform dan tes otomatis tertinggal. Review lebih baik, seperti yang ditulis oleh Agile Manifesto , hubungan lebih penting daripada proses atau alat . Tetapi begitu Anda mendapat ulasan, tes unit otomatis / uji regresi adalah bantuan terpenting berikutnya dalam menciptakan perangkat lunak yang baik.
Jika Anda dapat mendasarkan tes pada persyaratan , well, seperti yang dikatakan wanita di "When Harry Met Sally" , saya akan mendapatkan apa yang dia miliki!
Semua ulasan harus memiliki tempat parkir untuk menangkap persyaratan dan masalah desain pada tingkat pengkodean di atas. Setelah sesuatu diakui sebagai milik di tempat parkir, diskusi harus berhenti di ulasan.
Kadang-kadang saya pikir review kode harus seperti review skematik dalam desain perangkat keras - sepenuhnya publik, menyeluruh, tutorial, akhir proses, gateway yang setelah itu akan dibangun dan diuji. Tetapi tinjauan skematis merupakan hal yang berat karena mengubah benda fisik mahal. Tinjauan arsitektur, antarmuka, dan dokumentasi untuk perangkat lunak mungkin harus kelas berat. Kode lebih cair. Ulasan kode harus lebih ringan.
Dalam banyak hal, saya pikir teknologi adalah tentang budaya dan harapan seperti halnya alat khusus. Pikirkan semua improvisasi " Keluarga Swiss Robinson " / Flintstones / McGyver yang menyenangkan hati dan menantang pikiran. Kami ingin barang kami bekerja . Tidak ada jalan tunggal untuk itu, tidak ada "kecerdasan" yang entah bagaimana bisa diabstraksikan dan diotomatisasi oleh program AI tahun 1960-an .
sumber
Sebagian besar poin yang Anda uraikan hanyalah masalah pembentukan kode atau "permukaan":
Semua ini dapat diperiksa menggunakan beberapa alat otomatis : tidak perlu memiliki pengembang berpengalaman menghabiskan waktu melalui kode untuk mengawasi itu.
Saya tidak tahu sama sekali tentang .NET , tetapi, untuk PHP , kami memiliki alat untuk memeriksa hal-hal semacam itu; mengingat .NET sering dikatakan "lebih industri" daripada PHP, saya akan terkejut mendengar bahwa tidak ada alat untuk memeriksa hal semacam itu.
Alat otomatis dapat:
Surat dapat dikirim ke semua tim, atau orang yang melakukan kode yang tidak lulus tes - atau Anda dapat menggunakan beberapa antarmuka web pelaporan (catatan yang sama tentang .NET dan PHP)
Saya juga akan menambahkan bahwa pengujian otomatis dapat banyak membantu, untuk mendeteksi sejumlah kesalahan sebelum kode digunakan dalam produksi. Dan tes otomatis juga dapat membantu dengan beberapa metrik, saya kira.
Ulasan kode yang dilakukan oleh pengembang berpengalaman juga memiliki keuntungan besar lain yang tidak Anda bicarakan:
Tetapi untuk tinjauan kode yang lebih dalam dari sekedar pemformatan kode, Anda akan membutuhkan lebih dari setengah jam ...
sumber
Pengalaman saya dengan tinjauan kode adalah bahwa itu harus merupakan upaya gabungan untuk meningkatkan kode, bukan 'ukuran' untuk memutuskan siapa yang baik atau buruk dalam pekerjaan mereka. Ketika tidak masalah jika Anda mendapatkan banyak komentar selama ulasan kode Anda, pengulas akan lebih ketat, maka memberikan saran untuk meningkatkan kode.
Untuk meningkatkan kualitas kode check in, ditegakkan bahwa komentar ulasan diproses (biarkan reviewer menyetujui komentar yang diproses) dan juga menggunakan alat pengecekan kode statis untuk memaksa tingkat kualitas untuk komit awal.
sumber
Saya pikir sistem penilaian Anda adalah ide yang buruk. Apa intinya? Untuk mengidentifikasi programmer yang baik dan buruk? Setiap orang dalam tinjauan kode tersebut dapat membentuk penilaian tentang programmer tertentu berdasarkan kode yang disajikan dalam tinjauan kode lebih baik daripada beberapa penugasan nilai-nilai yang sewenang-wenang pada seperangkat karakteristik yang agak sewenang-wenang. Jika Anda ingin mengidentifikasi pemrogram yang baik dan buruk ... tanyakan pada pemrogram. Saya jamin manusia bisa melakukan penilaian ini lebih baik daripada heuristik konyol Anda.
Saran saya adalah mencoba dan meningkatkan ulasan kode sehingga orang berbagi ide dan pendapat secara terbuka di lingkungan yang tidak menghakimi dan tidak bermusuhan. Jika Anda bisa melakukan itu, Anda akan 100X lebih baik daripada memberikan penilaian pada programmer berdasarkan daftar periksa konyol Anda yang dimaksudkan untuk melakukan pekerjaan yang baik dalam menilai programmer. Saya pikir banyak programmer sudah bangga dan keras pada diri mereka sendiri jika mereka melakukan buruk dalam ulasan kode; Saya mempertanyakan apakah 'hukuman' lebih lanjut untuk kinerja yang buruk umumnya bermanfaat.
sumber
Satu-satunya saran saya adalah hindari membuat proses peninjauan kode Anda terlalu ketat - yang paling penting adalah bahwa peninjauan kode itu benar-benar terjadi dan itu ditanggapi dengan serius .
Semakin melelahkan proses bagi peninjau semakin kecil kemungkinannya bahwa ulasan kode akan terjadi dan bahwa mereka akan dianggap serius daripada hanya dilihat sebagai gangguan. Selain itu, nilai sebenarnya dari tinjauan kode terletak pada kemampuan pengulas untuk menggunakan penilaian mereka sendiri, alat otomatis dapat digunakan untuk memeriksa hal-hal seperti apakah aturan FXCop lulus.
sumber
Sebagai aturan praktis, hindari menghabiskan waktu dalam ulasan kode melakukan sesuatu yang bisa dilakukan oleh mesin. Misalnya, item pertama Anda adalah "menegakkan aturan FxCop", tetapi mungkin itu bisa dilakukan oleh FxCop tanpa manusia harus melakukannya juga.
sumber
Jika Anda dapat mengukurnya, jika objektif, dapat diukur, maka cobalah membuat alat melakukannya. Di mana Anda memerlukan resensi yang berpengalaman adalah untuk hal-hal subyektif fuzzy.
sumber
Banyak komentar yang baik telah dibuat tentang masalah gaya, yang penting. Pada proyek tim, sangat berharga untuk semua kode terlihat seperti itu ditulis oleh seorang penulis tunggal. Ini membuatnya lebih mudah bagi anggota tim lainnya untuk mampir dan memperbaiki masalah ketika itu terjadi. Apa ukuran kuantitatif yang Anda pilih untuk memastikan tujuan yang lebih luas ini kurang penting.
Satu item tambahan adalah memastikan bahwa kode cocok dengan arsitektur keseluruhan yang telah disepakati untuk seluruh sistem. Semua masalah yang serupa harus diselesaikan dengan cara yang sama. Jika logika aplikasi telah terpecah menjadi beberapa lapisan, apakah kode yang sedang ditinjau memecah fungsionalitasnya seperti yang dilakukan sistem lainnya? Atau, apakah kode yang sedang ditinjau mengajarkan sesuatu yang baru yang harus ditarik kembali ke seluruh sistem? Sama seperti pemeriksaan gaya memastikan kode semua terlihat sama, tinjauan arsitektur harus memastikan bahwa semua kode bekerja dengan cara yang sama. Penekanan di sini lagi adalah pada rawatan. Siapa pun di tim harus dapat masuk ke dalam kode ini dan memahami apa yang terjadi segera.
Ide penilaian tampaknya seperti bencana dalam pembuatan, tetapi Anda tahu tim Anda yang terbaik. Mungkin saja mereka akan termotivasi oleh sistem seperti itu, tetapi saya pikir lebih mungkin bahwa orang akan mulai lebih khawatir tentang nilai mereka daripada menyelesaikan masalah. Salah satu efek samping yang sangat berharga dari tinjauan kode adalah peluang mentoring yang mereka tawarkan. Peninjau harus memperlakukan orang yang menulis kode sebagai seseorang yang mereka bimbing. Setiap masalah yang ditemukan bukan masalah, tetapi kesempatan untuk membuat anggota tim yang lebih berpengetahuan dan canggih dan tim yang lebih erat secara keseluruhan.
sumber
Saya sebenarnya lebih peduli tentang hal-hal "subyektif" daripada yang lainnya, terus terang. Apa yang saya inginkan dari review kode yang baik adalah seseorang untuk memeriksa logika saya, bukan mengetik saya. Dan itulah yang saya fokuskan ketika saya memberikan ulasan kode.
Format umum yang ingin saya ambil adalah:
Tanpa itu, hanya melihat perbedaan cenderung memberi masukan pada masalah kecil atau poin gaya. Saya jauh lebih peduli dengan apakah logikanya benar, pendekatan yang digunakan secara keseluruhan baik, dan apakah solusinya akan dipertahankan.
Sebagai contoh, saya baru-baru ini melihat beberapa kode oleh rekan kerja. Masalah aslinya adalah pelanggaran FxCop. Tetapi yang sedang dilakukan adalah mencoba menentukan ada tidaknya fitur Windows dengan memeriksa nomor versi. Masukan utama saya adalah bahwa ini adalah cara yang rapuh untuk melakukannya, dan kami akan lebih baik menanyakan layanan secara langsung, karena pemetaan antara keberadaan fitur dan sku Windows dapat berubah di masa depan, dan sama sekali tidak tahan masa depan.
sumber
Kompleksitas Cyclomatic (CC) adalah salah satu cara untuk mengevaluasi kode yang 'tidak buruk'.
Dalam kode aktual yang memiliki CC tinggi, saya memiliki faktor "apa yang terjadi di sini, saya tidak ingat". Kode CC yang lebih rendah lebih mudah diketahui.
Jelas, peringatan yang biasa berlaku untuk metrik.
sumber
Ulasan kode bersifat subjektif dan objektif. Aturan seperti "badan metode harus 20-30 baris" bersifat subyektif (beberapa orang mungkin berpikir 100 baris OK) tetapi jika perusahaan Anda telah memutuskan bahwa 20-30 baris adalah batasnya, itu bagus dan Anda dapat mengukurnya. Saya pikir sistem poin yang Anda buat adalah ide bagus. Anda harus mengevaluasi ulang secara berkala karena Anda menemukan bahwa aturan tertentu harus memiliki bobot lebih atau kurang dalam penilaian tetapi selama semua orang tahu aturan, itu tampak seperti sistem yang baik.
Hal-hal lain yang akan saya cari:
sumber
Sepertinya Anda terlalu rinci terlalu cepat. Anda harus memecahnya lebih banyak. Anda harus mengamati kode untuk kualitas kode dan untuk kepatuhan fitur. Anda harus berpisah dan itu bahkan bukan akhir dari cerita ... jadi inilah saran saya:
Kualitas kode:
Kepatuhan fitur:
Jika Anda dapat menutupi diri Anda pada dua aspek dari tinjauan kode ini, Anda adalah emas.
sumber
Saya bisa menulis beberapa paragraf, tetapi saya hanya akan mengabaikan apa yang dijelaskan Karl Wiegers dalam "Peer Reviews in Software: A Practical Guide" . Saya pikir bukunya berisi jawaban yang jelas dan ringkas untuk pertanyaan Anda (dan banyak lagi).
sumber
Tergantung.
Beberapa bagian ulasan mudah diukur (tidak ada masalah FxCop, tidak ada kesalahan StyleCop , tidak ada kesalahan CAT.NET , dll.)
Gaya, bagaimanapun, bisa subjektif - tetapi seperti yang Anda katakan, begitu Anda mulai lebih spesifik (tidak ada metode> 20 baris) maka Anda dapat mengukurnya, dan alat-alat seperti NDepend dapat melakukannya secara otomatis. Beberapa hal tidak akan pernah otomatis - memeriksa penanganan tepi kasus akan memerlukan tes untuk melakukannya, yang memunculkan cakupan kode, dan 100% adalah ideal yang tidak terjangkau dalam banyak kasus. Pemeriksaan duplikasi sulit dilakukan secara otomatis. Tidak ada pemeriksaan, saya tidak yakin saya setuju dengan Anda, tetapi Anda mungkin bisa menulis aturan NDepend, atau aturan FxCop untuk yang itu.
Semakin banyak alat semakin baik, dan jika alat memungkinkan pengembang untuk memeriksa pekerjaan mereka sebelum melakukan perubahan dan untuk pemeriksaan dilakukan sebagai bagian dari proses CI maka Anda akan meminimalkan kebutuhan ulasan.
sumber
Sistem penandaan terdengar sulit untuk dilakukan dengan benar, tetapi bermanfaat untuk dimiliki sebagai alat pengukuran: Anda tidak dapat meningkatkan apa yang tidak dapat Anda ukur. Tetapi Anda mungkin harus menerima bahwa beberapa hal akan sulit / tidak mungkin untuk diukur secara akurat. Hal yang sulit adalah menentukan berapa banyak poin yang harus dinilai oleh setiap kualitas: misalnya, jika mematuhi konvensi penamaan skor 2 poin, lalu berapa banyak poin untuk menjaga metode kecil?
Mungkin sesuatu seperti daftar periksa sederhana akan lebih baik, sehingga kode dapat ditandai sebagai sesuai, sebagian sesuai atau tidak sesuai dengan kualitas tertentu. Kemudian, Anda dapat menambahkan penilaian ke daftar periksa setelah Anda melihat masalah kualitas apa yang paling sering muncul, atau paling banyak menyebabkan masalah.
Proses tinjauan juga harus cukup fleksibel untuk memungkinkan kode gagal bagian dari tinjauan, asalkan ini dapat dibenarkan dan didokumentasikan. Gagal menempel pada beberapa standar kualitas kode yang menyebabkan komponen menjadi rumit / tidak dapat dikelola adalah ide yang buruk!
sumber
Jika Anda ingin kode orang lebih terstandarisasi, tanpa membuatnya "buang waktu untuk memformat", karena beberapa pengembang akan mengeluh. Investasi dalam alat seperti ReSharper karena membuat memperbaiki pemformatan dan tugas re-factoring lainnya merupakan proses yang hampir otomatis.
sumber
sumber