Saya berada dalam posisi di mana saya telah diminta untuk meninjau beberapa kode yang memperbaiki masalah yang saya pikir tidak ada.
Pemecah masalah, yang lebih senior dari saya, menegaskan perbaikannya diperlukan tetapi tampaknya tidak lebih dari C + + sofistry kepada saya. Bagian dari proses penerapan kami adalah tinjauan kode, dan sebagai insinyur tertinggi ke-2 di perusahaan kecil saya diharapkan untuk meninjau perubahan.
Saya percaya bahwa pengulas sama bertanggung jawab atas perubahan kode seperti pembuat kode asli, dan saya tidak mau menerima tanggung jawab atas perubahan ini. Bagaimana cara Anda menolak ulasan ini?
code-reviews
James
sumber
sumber
size > size+1
untuk memeriksa overflow pada integer yang ditandatangani, kompiler dapat dengan mudah mengganti ekspresi ini denganfalse
, karena overflow dari integer yang ditandatangani tidak ditentukan. Lihat blog.llvm.org/2011/05/…Jawaban:
Mintalah uji kasus yang gagal tanpa perubahan yang berhasil dengan perubahan tersebut.
Jika dia tidak bisa menghasilkan satu, Anda menggunakannya sebagai pembenaran.
Jika dia dapat menghasilkan satu maka Anda perlu menjelaskan mengapa tes tidak valid.
sumber
Peninjau harus objektif.
Jelas bahwa Anda telah membentuk pendapat tentang kode yang dipermasalahkan bahkan sebelum Anda meninjaunya, dan sepertinya Anda dan pemecah masalah telah mempertaruhkan posisi. Jika demikian, maka Anda akan mengalami kesulitan untuk tampil objektif, dan bahkan lebih sulit lagi menjadi objektif. Tidak ada yang membantu prosesnya, dan mungkin hal terbaik dan paling obyektif yang dapat Anda lakukan adalah mengundurkan diri dengan alasan Anda terlalu dekat dengan masalah tersebut.
Pertimbangkan pendekatan tim.
Jika Anda tidak dapat menghapus diri sendiri, mungkin Anda dapat meminta beberapa insinyur lain meninjau kode secara bersamaan. Entah mereka akan setuju dengan Anda bahwa kode tersebut harus ditolak atau tidak. Jika mereka setuju dengan Anda, maka itu tidak lagi hanya Anda vs pemecah masalah, dan Anda akan dapat membuat kasus yang lebih kuat bahwa tim melihat perbaikan secara objektif dan memutuskan untuk tidak menerimanya. Di sisi lain, jika mereka memutuskan untuk menerima perbaikan maka itu juga akan menjadi keputusan tim. Itu harus berjalan tanpa mengatakan bahwa Anda harus berpartisipasi dengan pikiran yang terbuka seperti yang Anda bisa kelola, dan bahwa Anda tidak boleh mencoba untuk mempengaruhi pendapat anggota tim lain dengan apa pun selain diskusi rasional. Penting: jika ada hasil yang buruk nanti, jangan lempar tim ke bawah bus dengan mengatakan "Ya, saya selalu mengatakan itu kode yang buruk, tetapi saya kalah jumlah dengan anggota tim lainnya. "
Penolakan adalah bagian alami dari proses peninjauan kode.
Proses peninjauan kode tidak ada untuk memperbaiki stempel karet dari orang yang lebih senior; itu ada di sana untuk melindungi dan meningkatkan kualitas kode. Tidak ada yang salah dengan menolak perbaikan asalkan Anda melakukannya karena alasan yang benar, yaitu bahwa perbaikan tidak meningkatkan kode. Jika, setelah peninjauan terbuka terhadap kode, Anda masih merasa bahwa perbaikan tidak mengurangi risiko dan / atau besarnya masalah yang dapat dibuktikan, maka Anda harus menolaknya. Ini bukan pribadi, hanya pendapat jujur Anda. Jika fixer tidak setuju, tidak apa-apa juga, dan pada saat itu menjadi masalah bagi manajemen untuk mencari tahu. Pastikan untuk tetap jujur, terbuka, dan profesional.
Tanggung jawab memotong kedua cara.
Anda mengatakan bahwa Anda tidak ingin bertanggung jawab atas perubahan ini, tampaknya karena Anda tidak percaya bahwa ada masalah. Namun, Anda perlu menyadari bahwa jika Anda salah dan ada adalah masalah, maka Anda mungkin berakhir menjadi bertanggung jawab untuk menolak kode yang akan menghindari masalah.
Ambil catatan.
Menyimpan catatan tertulis dari proses peninjauan akan membantu Anda menjaga fakta tetap benar. Tuliskan pikiran dan kekhawatiran Anda saat meninjau, menguraikan, dan hasil dari setiap tes yang mungkin Anda jalankan untuk mengukur dugaan masalah dan perbaikannya, dll. Jika masalah ini meningkat, Anda akan memiliki catatan tentang apa yang telah Anda lakukan untuk mendukung Anda posisi. Jika masalah muncul lagi di masa mendatang (mungkin akan terjadi jika fixer melekat pada pandangannya sendiri), Anda akan memiliki sesuatu untuk mengacaukan memori Anda.
sumber
Tuliskan alasan Anda untuk penolakan dan pembelaan untuk kemungkinan kontra argumen. Kemudian diskusikan secara rasional dengan pemecah masalah dan jika perlu tingkatkan ke manajemen.
Jika Anda memiliki dokumentasi yang memadai (termasuk daftar kode, hasil pengujian, dan argumen objektif ), Anda akan tercakup dengan baik.
Anda akan menyebabkan beberapa niat buruk dengan pemecah masalah, jadi pastikan masalah ini sepadan (yaitu, apakah penyebab tidak diperbaiki membahayakan?)
Juga, jika pemecah masalah terkait dengan pemiliknya, lupakan jawaban ini.
sumber
Baru-baru ini di berita LinkedIn, ada sebuah artikel tentang resolusi konflik di tempat kerja dan menjadi rendah hati, bukannya terlihat arogan. Sayangnya, saya tidak dapat menemukan tautannya sekarang, tetapi intinya adalah mencoba membuat pertanyaan dengan cara yang tidak konfrontatif. Tolong jangan menganggap ini sebagai saya menyiratkan bahwa Anda sombong, itu hanya cara di mana artikel itu ditulis.
Bagaimanapun, dalam memberi tahu programmer senior bahwa dia salah dalam asumsinya hanya akan membuatnya mengambil pendekatan defensif dan menyerang Anda kembali dalam tanggapannya. Namun, jika Anda mengajukan pertanyaan mengapa ia percaya masalah itu ada, dari sudut pandang kurangnya pemahaman Anda, maka dia cenderung lebih terbuka untuk membahas masalah tersebut.
Jadi, daripada mengatakan "Masalahnya tidak ada, jadi saya tidak harus meninjau ini", mungkin bertanya sesuatu seperti "Untuk meninjau ini dengan benar, saya perlu memahami masalahnya. Bisakah Anda menjelaskannya dari Anda?" sudut pandang?"
Sebagai contoh, artikel tersebut menggambarkan tes yang diberikan kepada anak-anak di mana orang dewasa mengangkat kubus yang wajahnya semua satu warna, kecuali yang menghadap orang dewasa. Ketika anak-anak ditanya warna wajah apa yang dilihat orang dewasa, mereka yang berusia di bawah 5 tahun hampir selalu mengatakan warna yang dapat mereka lihat, karena mereka tidak dapat memahami bahwa orang dewasa dapat memiliki pandangan yang berbeda dengan wajah mereka.
sumber
C / C ++ bisa penuh dengan perilaku yang tidak terdefinisi. Satu sisi itu buruk karena dapat menyebabkan perilaku yang tidak terduga. Di sisi lain itu memungkinkan pengoptimalan agresif dan biasanya ketika Anda menggunakan C / C ++ Anda tertarik pada kecepatan.
Menulis test case yang rusak mungkin sulit - mungkin melibatkan arsitektur aneh atau kompiler yang tidak ada lagi. Juga mungkin tampak seperti pada arsitektur yang masuk akal "itu seharusnya tidak menyebabkan masalah".
Namun pada satu titik atau lain Anda akan mengubah platform (katakan - Anda ingin mobile sehingga Anda porting ke ARM atau Anda ingin mempercepat dan menggunakan GPU). Pada titik ini hal-hal mungkin mulai rusak dan Anda perlu men-debug itu. Mungkin sepele seperti memperbarui kompiler ke versi yang lebih baru (dan Anda mungkin menginginkannya / membutuhkannya).
Kode yang bermasalah adalah:
Selama iterasi terakhir
d[++k] => d[++15] => d[16]
diakses. Karena biasanya elemen berikutnya adalah memori legal (dalam hal paging bukan model memori C) bahkan kompiler sepele tidak menghasilkan executable dengan perilaku aneh. Satu-satunya cara menulis kasus uji adalah menemukan platform dengan model memori yang persis sama dengan C (mungkin VM).Namun gcc 4.8 prerelase terlihat yang
d[++k]
dieksekusi di setiap loop. Jadik < 16
kalau tidak, akses akan ilegal dan legalitas program yang dimasukkan ke kompiler adalah bagian dari kontrak. Jadi kondisi loop selalu benar mengingat asumsi sehingga loop tak terbatas. Ini mungkin terdengar aneh tapi itu optimasi yang sangat legal - memancarkansystem("dd if=/dev/zero of=/dev/sda"); system("format c:")
juga akan menjadi pengganti yang tepat untuk loop. Ada cara yang lebih halus yang bisa Anda pilih untuk menunjukkan perilaku. Sebagai contoh selama kuliah tentang Memori Transaksional saya ingat bahwa presenter mencoba beberapa kali untuk mendapatkan nilai yang salah ketika 2 utas meningkatkan nilai yang sama.Namun C / C ++, berlawanan dengan beberapa bahasa, distandarisasi sehingga perselisihan tersebut dapat dilakukan dengan mengacu pada sumber obyektif:
Secara umum jika tim Anda menulis dalam C / C ++, sangat berguna untuk memiliki standar - bahkan pakar tim dapat menemukan sesuatu yang aneh sesekali.
sumber
Ini terdengar lebih seperti masalah dengan kebijakan tim Anda daripada dengan ulasan khusus ini. Ketika saya bekerja di sebuah toko perangkat lunak besar di sebuah tim yang terdiri dari 9 orang, seorang flat-out reviewer memiliki hak veto atas kode, dan ini adalah standar yang kita semua hormati. Kami cukup berbakat dan cerdas - yaitu. "dewasa", tetapi lebih dalam arti "tidak kekanak-kanakan" daripada "berpengalaman" - bahwa pimpinan tim kami dapat secara wajar mengharapkan kami untuk selalu dapat mencapai kesepakatan tanpa beralih ke argumen dalam periode waktu yang bijaksana.
Hanya berdasarkan pada bahasa Anda di pos Anda, saya mungkin memperingatkan Anda bahwa sepertinya Anda akan mendekati situasi ini dengan cara yang dapat mengarah pada argumen devolusi. Mungkin itu adalah "masturbasi intelektual," tetapi mungkin ada alasan dia melakukannya, dan beban Anda adalah menemukan cara yang lebih sederhana untuk menyelesaikan masalah itu - dan jika tidak ada cara seperti itu, catat dalam komentar mengapa kode masturbasi sebenarnya diperlukan.
sumber
Buat bukti acara submitter bahwa kodenya memperbaiki masalahnya. Jika dia bisa menguji, dan menunjukkan bukti kepada Anda, maka Andalah yang salah dan harus berhenti dari keluhan Anda dan mengatasinya. Jika dia tidak dapat menunjukkan bukti untuk mendukung klaimnya ada masalah, dan menunjukkan bukti bahwa tambalannya memperbaiki masalah, maka saya akan mengirimnya kembali ke papan gambar.
Tentu saja mungkin ada kepekaan politis internal di sekitar ini. Tapi ini adalah cara saya pergi (enak).
sumber