Apa cara terbaik bagi programmer untuk menangani kode yang ditinjau?

16

Saya cukup baru dalam review kode, tetapi saya telah mengkode selama bertahun-tahun selama PhD - yang tidak selalu membuat Anda seorang programmer yang baik!

Jika reviewer mengubah kode Anda dan melewatinya dengan Anda baris demi baris, apa yang Anda lakukan jika Anda tidak setuju? Kadang-kadang Anda membuat pilihan X dan peninjau mengubahnya menjadi Y, dan Anda memiliki Y di pikiran Anda tetapi memutuskan untuk tidak melakukannya karena kerugiannya, tetapi pengulas menyatakan bahwa kerugian itu tidak penting. Haruskah Anda menyatakan ketidaksetujuan Anda, atau tidak dan mendengarkan mereka?

Sulit jika Anda tidak pandai menerima kritik, dan jika peninjau adalah orang yang lebih senior dalam organisasi. Tidak baik jika dianggap terlalu defensif.

Paul Richards
sumber
1
berdiskusi, itu bukan ulasan jika itu hanya lalu lintas satu arah
NimChimpsky
@gnat: Pertanyaan itu jelas bukan duplikat. Lihatlah jawaban yang terpilih, diterima untuk pertanyaan itu. Jika itu diberikan sebagai jawaban di sini, itu akan diturunkan, karena tidak menjawab pertanyaan ini .
Michael Shaw
@gnat: Pertanyaan lain adalah tentang bagaimana cara mendapatkan kode orang lain ditolak dalam ulasan, dan yang ini adalah tentang bagaimana menangani kritik terhadap kode sendiri dalam ulasan. Satu-satunya kemiripan yang bisa saya lihat adalah bahwa keduanya melibatkan ulasan kode.
Michael Shaw

Jawaban:

19

Benar, tampil defensif tidak membantu; tetapi kemudian - tidak ada yang dikritik, jadi jika Anda merasa ini terjadi, Anda benar-benar harus menyuarakan kekhawatiran Anda bahwa tinjauan kode tidak membantu Anda menulis kode yang lebih baik dalam organisasi.

Peninjau memiliki tanggung jawab untuk meninjau kode untuk ketidakpatuhan nyata dan cacat, tidak menggunakannya sebagai sarana untuk menulis kode Anda seperti yang seharusnya mereka lakukan. Ini berarti peninjauan ada di sana untuk meninjau bagaimana Anda melakukan sesuatu, dan tunjukkan setiap bidang yang Anda lakukan kesalahan (sesuatu yang terbaik dari kita lakukan) atau tidak memahami kerangka kerja atau standar atau "alasan historis" beberapa kode dituliskan. cara itu adalah tempat Anda berada.

Jadi, jika ada banyak cara melakukan sesuatu, perlu ada alasan kuat mengapa kode Anda diubah ke cara alternatif, jika tidak, itu hanya churn non-konstruktif yang tidak akan membantu Anda.

Juga, ingatlah bahwa resensi buku juga tidak sempurna. Mereka mungkin memiliki gagasan bahwa Y adalah cara untuk melakukannya, dan belum menyadari bahwa X lebih baik. Anda harus menjelaskan alasan mengapa Anda melakukannya dengan cara X. Peninjau mungkin setuju dengan Anda, atau dia mungkin memberi tahu Anda mengapa Y adalah solusi yang lebih baik - mungkin ada faktor lain yang tidak Anda ketahui.

Singkatnya, ulasan adalah cara untuk membuat anggota tim berkomunikasi tentang perubahan kode mereka. Jadi bicaralah dengan pengulas tentang hal itu.

JIKA itu membantu, bicaralah dengan cara yang tidak memihak, seolah-olah Anda bahkan bukan pembuat kode yang ditinjau. Bagaimanapun, kode itu milik perusahaan atau tim, dan tim harus memeliharanya. Anda kebetulan adalah orang yang menulisnya, itu bukan faktor yang penting seperti banyak orang percaya.

gbjbaanb
sumber
7
"Peninjau memiliki tanggung jawab untuk meninjau kode untuk benar-benar ketidakpatuhan dan cacat, tidak menggunakannya sebagai sarana untuk menulis kode Anda seperti yang seharusnya mereka lakukan.": +1 untuk menunjukkan ini.
Giorgio
+1 "Ulasan adalah cara untuk membuat anggota tim berkomunikasi tentang perubahan kode mereka"
Kwebble
20

Menulis kode komputer adalah contoh utama membuat keputusan di bawah ketidakpastian . Selalu ada tekanan yang saling bertentangan, dan bagaimana Anda memutuskan dalam setiap pertanyaan yang diberikan tergantung pada tekanan apa yang Anda rasakan dan seberapa besar Anda menganggapnya.

Karena itu, ketika pengulas tidak setuju dengan keputusan Anda, itu berarti mereka melihat tekanan / risiko yang berbeda dari Anda, atau mereka menganggap beberapa di antaranya lebih besar / lebih kecil daripada Anda. Anda harus benar - benar berbicara tentang perbedaan-perbedaan ini, karena tidak melakukan hal itu akan melanggengkan masalah yang menyebabkan perselisihan pada awalnya.

Jika pengulas Anda lebih senior, pengalaman mereka mungkin dengan benar mengatakan kepada mereka bahwa risiko ini atau itu tidak terlalu relevan dalam praktiknya, atau mereka mungkin tahu bahwa beberapa jenis kesalahan memiliki sejarah panjang terjadi di organisasi Anda, dan ada baiknya ekstra hati-hati untuk menghindarinya. Sebaliknya, jika Anda merasa mengetahui sesuatu yang mungkin tidak diketahui oleh pengulas Anda, Anda harus benar-benar mengungkapkannya; menjaga jumlah diam untuk melalaikan tugas di pihak Anda.

Bagian paling penting dari penanganan situasi adalah untuk memahami bahwa kritik terhadap keputusan desain hampir selalu bukan kritik terhadap kepribadian seseorang. (Dalam kasus langka di mana itu sebenarnya, Anda akan melihat itu segera, dan jika Anda benar-benar tidak dapat menyenangkan seseorang, tidak ada yang Anda lakukan membuat perbedaan, jadi di mana salahnya mengikuti praktik terbaik? Jauh lebih baik untuk menemukan posisi yang lebih baik sebagai sesegera mungkin.) Ini hanya hasil dari orang yang berbeda yang memiliki persepsi berbeda tentang banyak risiko dan imbalan yang terlibat dalam kode komputer, dan mengingat betapa rumitnya kode komputer modern, hanya itu yang diharapkan. Membicarakan perbedaan membantu meningkatkan kode dan meningkatkan kerja sama Anda dalam kasus ini dan dalam kasus mendatang.

Kilian Foth
sumber
4

Jawaban lain sudah berisi informasi yang sangat bagus. Saya ingin sedikit memperluas beberapa aspek yang telah diisyaratkan oleh gbjbaanb (lihat komentar saya untuk jawabannya).

Dalam pengalaman saya, saya telah mengamati berbagai jenis umpan balik selama ulasan kode:

  1. "Jujur saya percaya bahwa ini salah / suboptimal dan bahwa Anda harus mengubahnya dengan cara ini." Saya biasanya menerima umpan balik semacam ini dengan serius, dan saya hanya akan menentang perubahan yang disarankan jika saya pikir saya memiliki poin kuat menentangnya.
  2. "Saya menemukan solusi Anda baik-baik saja, pertimbangkan alternatif ini tetapi terserah Anda apakah Anda menerima perubahan atau tidak." Saya menanggapi umpan balik semacam ini dengan sangat serius: pengulas menyarankan sebuah alternatif, meskipun mereka tidak memiliki pendapat yang kuat bahwa solusi mereka lebih baik. Saya memiliki kesempatan untuk mempelajari sesuatu dan saya akan menerima perubahan jika saya lebih suka. Kalau tidak, pengulas menemukan itu OK jika saya menyimpan kode sesuai selera saya.
  3. "Saya akan melakukannya secara berbeda sehingga Anda harus mengubahnya, titik.", Atau bahkan "Oh, saya telah mengubah kode Anda karena ...", perubahan itu tidak disarankan, sudah dilakukan! Beberapa penjelasan singkat tentang perubahan diberikan, dengan petunjuk bahwa tidak ada banyak waktu untuk membahas detailnya karena kita harus beralih ke tugas berikutnya.
  4. Peninjau menyarankan perubahan kecil yang sifatnya sepele yang tidak memperbaiki kode tetapi hanya membuatnya berbeda. Ketika diminta untuk membahas perubahan yang disarankan, pengulas melakukan diskusi panjang tentang rincian sepele sampai Anda menyerah karena kelelahan.

Opsi 3, 4 dapat disamarkan sebagai 1 dan 2: "Sangat penting untuk melakukannya seperti yang saya sarankan." atau "Anda dapat menolak perubahan jika Anda benar-benar menginginkannya.", berkata dengan nada yang sebenarnya berarti kebalikan dari apa yang dikatakan. Jika Anda menentang perubahan yang Anda anggap tidak perlu, kepemilikan kode bersama dapat digunakan sebagai pembenaran atas sikap ini: "Kode itu bukan milik Anda, itu milik tim!" Anda dapat mengetahui kapan niat peninjau tidak jujur ​​jika peninjau tidak terlalu terbuka untuk diskusi, merasa jengkel dan mencoba mendorong solusi mereka dengan cara apa pun. Pada dasarnya mereka sudah memutuskan, dan diskusi lebih lanjut hanya membuang-buang waktu.

Opsi IMO 1 dan 2 adalah tanda pengulas jujur ​​yang mencoba membantu, mencoba mengajari Anda sesuatu sambil menghormati pekerjaan Anda, dan juga terbuka untuk mempelajari sesuatu sendiri. Dalam skenario ini saya mencoba untuk tidak terlalu bangga ketika saya mendapatkan umpan balik yang membangun dari pengulas.

Opsi 3, 4 menyarankan bahwa ada beberapa permainan kekuatan yang sedang terjadi: yang penting adalah apakah kita melakukannya dengan cara saya atau cara Anda, bukan bahwa kami mencoba mencari solusi yang baik yang memuaskan keduanya. Ini mungkin terkait dengan ego peninjau, tetapi juga karena mereka tidak dapat memahami kode apa pun yang tidak mengikuti cara berpikir mereka. Mereka biasanya terganggu oleh kode yang tidak tampak akrab bagi mereka dan ingin memaksakan jalan mereka pada seluruh basis kode.

Jika situasi 3 dan 4 terjadi terlalu sering, kerja tim bisa menjadi sangat tidak menyenangkan. Dalam situasi seperti itu saya akan mempertimbangkan untuk meninggalkan tim.

Giorgio
sumber
Saya telah menemukan bahwa jika saya pernah merasa menjadi 3 atau 4, saya harus menunjukkan apa yang mereka lakukan benar-benar rusak ("Lihat, ini sebenarnya gagal jika nama belakang pelanggan adalah Null") atau menulis kedua solusi, dan periksa untuk melihat apakah solusi yang saya usulkan benar-benar layak untuk diubah (mungkin kode saya lebih mudah dibaca tetapi lebih lambat, atau sebaliknya, dalam hal ini biasanya saya tidak akan repot-repot membaca perubahan, kecuali perbedaannya signifikan (baik dalam keterbacaan atau kecepatan)).
scragar
@scagar: Benar: seseorang selalu berusaha untuk berpegang pada fakta. Namun, kadang-kadang bisa melelahkan. Contoh (dalam konteks komit yang agak luas): Anda harus membandingkan std :: string dengan QString. Solusi Anda: Konversikan std :: string ke QString dan gunakan metode QString untuk membandingkan. Perubahan reviewer: konversi QString ke std :: string dan gunakan metode std :: string untuk membandingkan. Anda dapat memikirkan variasi tak terbatas tentang cara mengubah kode menjadi kode yang setara hanya untuk menunjukkan bahwa Anda telah berada di sana. Sangat penting untuk membedakan antara umpan balik konstruktif dan nitpicking.
Giorgio
3

Untuk mengatasi masalah Anda tentang apa yang harus dilakukan ketika Anda tidak setuju ...

Berbicara itu adalah cara kita untuk pergi karena kebanyakan orang sudah menunjukkan.

Jika Anda sudah melakukan itu, mungkin lebih dari sekali saja, satu teknik berguna yang kami gunakan adalah jika masih ada ketidaksepakatan di beberapa bidang adalah dengan mengatakan 'ya, akan lebih baik untuk menolak itu -

Bisakah kita memasukkan tiket terpisah untuk itu?

Tiket terpisah memungkinkan Anda memasukkan kode, alih-alih mode 'churn and never move on' yang ditakuti yang saya kenal baik di beberapa tempat. Ini cocok dengan lincah, melakukan jumlah terkecil 'mungkin' dan menyebarkan beban. Terkadang yagni akan berakhir melamar. Beberapa kali manajer produk akan memutuskan bahwa ada kebutuhan yang lebih mendesak daripada refactor dalam hal nilai untuk klien, tenggat waktu dan sumber daya.

Michael Durrant
sumber
1

Tinjauan kode mungkin adalah hal yang baik secara umum, tetapi dari pengalaman saya, yang terbaik adalah berfungsi sebagai alat untuk pengembang di tim baru menggunakan pedoman pengkodean baru atau untuk memperbaiki bug penting, sehingga risiko melakukan kesalahan akan berkurang. Jika Anda percaya bahwa Anda lebih tahu daripada pengulas, Anda harus bertanya mengapa solusi yang ia usulkan lebih baik dan berdebat dengan wawasan Anda tentang masalah tersebut. Tidak ada yang tahu segalanya yang terbaik, jadi tinjauan kode harus atau bisa menjadi pengalaman belajar yang berharga bagi keduanya.

cseder
sumber
1

Peninjauan kode merupakan kesempatan untuk menangkap masalah potensial dan menyampaikan pengetahuan, baik untuk peninjau maupun pembuat kode.

Sebagai peninjau kode, tanggung jawabnya adalah menyoroti bidang-bidang risiko yang mungkin, ketidaksesuaian dengan praktik standar, peningkatan, dan umumnya hanya sudut pandang lain pada bidang kode yang sama.

Ini seharusnya tidak menghasilkan perubahan kode tanpa mendiskusikan / memahami keputusan coders pada saat pengembangan.

Jika pengulas membuat perubahan, mereka mungkin mengalami kesulitan mendelegasikan pekerjaan kepada orang lain, yang sulit dilakukan oleh banyak orang pintar.

Sebagai pembuat kode yang menerima ulasan, Anda dilindungi dari kemungkinan masalah saat digunakan, diberi kesempatan untuk mempelajari sesuatu yang baru, dan kesempatan untuk berbagi pengetahuan Anda dengan pengulas.

Terlepas dari senioritas cara berpikir Anda dapat menghasilkan solusi yang tidak terpikirkan oleh sebagian orang, sehingga ulasan dapat menjadi kesempatan Anda untuk bersinar hanya dengan melakukan apa yang Anda yakini benar.

Selama baik pembuat kode dan peninjau menerima bahwa mereka bisa salah, dan bahwa boleh saja itu salah, ulasan menjadi sesuatu yang memperkuat tim karena Anda bekerja bersama dalam solusi.

Kevin Hogg
sumber
0

Sepertinya kode Anda belum ditinjau :-)

Tujuan dari tinjauan kode adalah untuk mendapatkan kode dalam kualitas yang layak, dan untuk mengetahui bahwa Anda telah mendapatkan kode dengan kualitas yang layak. Ketika kode pengembang yang tidak berpengalaman ditinjau, maka dapat digunakan untuk mengajarkan cara menulis kode yang lebih baik, sambil menghindari membuat frustrasi pengembang itu.

Peninjau seharusnya tidak pernah mengubah kode Anda. Mereka dapat membuat saran yang kurang lebih kuat bagaimana mereka ingin kode Anda diubah, dan mereka dapat memutuskan apakah akan menerima kode Anda atau tidak.

Jika ulasan berjalan dengan benar / jika saya meninjau kode Anda, apa yang mungkin akan Anda dapatkan adalah beberapa komentar bagaimana saya akan menulis kode yang dapat Anda pelajari, atau abaikan - ini adalah hal-hal di mana saya memiliki pendapat dan Anda bebas untuk memiliki pendapat berbeda. Di daerah saya, penamaan fungsi yang baik, variabel, dan sebagainya dianggap penting, sehingga Anda dapat memperoleh beberapa saran untuk meningkatkan penamaan. Biasanya Anda harus membuat perubahan dalam kasus itu (kadang-kadang dengan mencari nama yang lebih baik untuk sesuatu). Terkadang saya akan menemukan bug. Anda memperbaikinya. Terkadang saya menemukan hal-hal yang saya anggap bug, dan saya salah. Jika sulit untuk melihat bahwa kode itu benar, Anda membuatnya lebih jelas. Jika saya salah, Anda memberi tahu saya.

Jika saya berpikir bahwa desainnya secara umum tidak benar, maka ini seharusnya sudah dibahas sebelumnya. Kami kemudian harus berpikir tentang apakah desain Anda cukup baik, dengan mempertimbangkan berapa banyak pekerjaan yang terlibat dalam suatu perubahan, atau mungkin saya hanya salah dan desain Anda lebih baik. Kita harus berakhir dengan kesepakatan.

Jika pengulas dan penerima ulasan tidak setuju, maka kami memiliki masalah. Karena itu berarti bahwa salah satu dari kita tidak mampu melakukan kerja tim, atau salah satu dari kita tidak dapat membedakan antara desain yang baik atau buruk, atau keduanya. Ini belum tentu salahmu. Sayangnya ada pengembang yang senior dan tidak mengerti, dan itu akan menjadi masalah bagi perusahaan dan untuk Anda.

Jika itu terjadi, pikirkan sangat, sangat keras: Apakah Anda memiliki masalah dalam menerima kritik yang beralasan? Jika itu masalahnya, Anda perlu mengubah sikap Anda. Apakah Anda terlalu berpengalaman untuk melihat mengapa peninjau itu benar? Jika itu masalahnya, itu tidak masalah. Percayai pengulas dan pelajari. Apakah Anda yakin tahu lebih baik daripada pengulas? Terima ulasan, tetapi tanyakan pengembang ketiga yang tepercaya tentang pendapatnya. Ingat Anda bisa benar-benar yakin pada diri sendiri dan benar, tetapi Anda juga bisa benar-benar yakin pada diri sendiri dan salah.

gnasher729
sumber