แอนติแพตเทิร์นของการรีวิวโค้ด

• การรีวิวโค้ดฟังดูเป็นไอเดียที่ดีใช่ไหม?
  • การรีวิวโค้ดเปิดโอกาสให้นักพัฒนาสองคนดูโค้ดร่วมกัน ค้นหาปัญหา และแบ่งปันความเข้าใจเกี่ยวกับพัฒนาการของโปรเจ็กต์
  • ผู้รีวิวอาจได้เรียนรู้เทคนิคที่มีประโยชน์จากการดูโค้ดของผู้เขียนอย่างละเอียด หรือพบโอกาสในการบอกเทคนิคที่มีประโยชน์ให้ผู้เขียน
• แต่นั่นคือวิธีที่ผู้รีวิวโค้ดฝั่ง 'Light Side' ใช้กัน นั่นคือใช้การรีวิวโค้ดเพื่อปรับปรุงโค้ดและยกระดับทักษะโดยรวมของนักพัฒนา
• การรีวิวโค้ดยังอาจเป็นเครื่องมือทรงพลังสำหรับจุดประสงค์ที่ต่างออกไปโดยสิ้นเชิง หากผู้รีวิวหันไปสู่ 'Dark Side' ก็สามารถใช้วิธีต่างๆ เพื่อขัดขวางหรือทำให้การปรับปรุงโค้ดล่าช้าได้
  • รวมถึงอาจใช้เพื่อเป้าหมายส่วนตัวอื่นๆ เช่น ก่อกวนผู้เขียนแพตช์หรือทำให้หมดกำลังใจไปเลย
• ถ้าคุณเพิ่งเปลี่ยนไปอยู่ฝั่ง 'Dark Side' เมื่อไม่นานนี้ คุณอาจยังนึกไม่ออกว่ามีความเป็นไปได้อะไรบ้าง
  • ดังนั้นบทความนี้จึงนำเสนอรายการแอนติแพตเทิร์นสำหรับผู้รีวิวโค้ดสายมืด

[แอนติแพตเทิร์น]

The Death of a Thousand Round Trips

• ระหว่างอ่านโค้ด พอเจอเรื่องเล็กน้อยก็รีบเขียนคอมเมนต์รีวิวทักทันที แล้วหยุดอ่านตรงนั้นเลย
• นักพัฒนาก็ขยันแก้เรื่องเล็กน้อยนั้นแล้วส่งแพตช์เวอร์ชันใหม่มา
• ผู้รีวิวเริ่มอ่านเวอร์ชันนั้น แล้วก็เจอเรื่องเล็กน้อยอีกอย่างหนึ่ง ทั้งที่จริงพูดได้ตั้งแต่รีวิวรอบแรกแต่ไม่ได้พูด จากนั้นก็ทักเรื่องนั้นแล้วหยุดอ่านอีก
• ทำแบบนี้ซ้ำไปเรื่อยๆ จนนักพัฒนาหมดหวัง

The Ransom Note

• แพตช์นี้ดูเหมือนจะสำคัญกับนักพัฒนาเป็นพิเศษ
• แต่สำหรับผู้รีวิว มันไม่ได้สำคัญขนาดนั้น ดังนั้นผู้รีวิวจึงอยู่ในตำแหน่งที่มีอำนาจ
• ผู้รีวิวสามารถจับการเปลี่ยนแปลงที่นักพัฒนาต้องการเป็นตัวประกันไว้ จนกว่างานเพิ่มเติมที่สำคัญต่อผู้รีวิวจะเสร็จสิ้น
  • งานเพิ่มเติมนั้นจริงๆ ไม่จำเป็นต้องอยู่ในคอมมิตเดียวกันก็ได้ แต่มันเป็นงานที่สำคัญสำหรับผู้รีวิว

The Double Team

• หนึ่งแพตช์ สองผู้รีวิว
• ทุกครั้งที่ผู้รีวิวคนหนึ่งขอให้แก้ นักพัฒนาก็ยอมแก้ตามดีๆ แล้วก็ถึงตาของผู้รีวิวอีกคนที่จะบ่น
• ผู้รีวิวผลัดกันตั้งข้อเรียกร้องที่ขัดแย้งกัน
  • แต่จะคอมเมนต์ใส่นักพัฒนาเสมอ หลีกเลี่ยงการโต้เถียงกันตรงๆ ในเธรดรีวิว
• เป้าหมายคือดูว่าผู้รีวิวจะโยนนักพัฒนาไปมาได้กี่รอบก่อนที่เขาจะยอมแพ้

The Guessing Game

• มีปัญหาอยู่หนึ่งอย่าง และมีหลายวิธีที่จะใช้แก้ปัญหานั้น
• นักพัฒนาเลือกวิธีแก้แบบหนึ่งแล้วส่งแพตช์มา
• ผู้รีวิววิจารณ์วิธีแก้นั้นด้วยเหตุผลที่ไม่เกี่ยวกับว่ามันแก้ปัญหาได้จริงหรือไม่
  • เช่น อ้างว่าละเมิดหลักการออกแบบที่คลุมเครือ หรือไม่สอดคล้องกับแผนการพัฒนาในอนาคต
• ถ้าวิจารณ์ให้คลุมเครือพอ นักพัฒนาก็จะไม่รู้ว่าควรแก้แพตช์เดิมอย่างไรเพื่อคลี่คลายข้อวิจารณ์นั้น
  • สำหรับนักพัฒนา วิธีที่ดีที่สุดจึงอาจเป็นการเลือกวิธีแก้ปัญหาแบบอื่นไปเลย แล้วลองทำอันนั้นแทน
• จากนั้นผู้รีวิวก็ปฏิเสธมันอีกแบบเดิมที่ไม่ช่วยอะไร

The Priority Inversion

• ในการรีวิวโค้ดรอบแรก ชี้เรื่องเล็กๆ น้อยๆ และแก้ง่ายก่อน
• รอให้นักพัฒนาแก้สิ่งเหล่านั้นเสร็จ แล้วค่อยหยิบยกปัญหาใหญ่ขึ้นมา
  • มีปัญหารากฐานที่ทำให้ต้องเขียนส่วนสำคัญของแพตช์ใหม่เกือบทั้งหมด ซึ่งหมายความว่างานแก้เรื่องเล็กน้อยที่ทำไปก่อนหน้านี้จำนวนมากต้องถูกทิ้ง
  • ไม่มีอะไรสื่อว่า “เราไม่ต้องการงานของคุณ และเวลาของคุณไม่มีค่า” ได้ดีไปกว่าการทำให้ใครสักคนลงแรงมากมายแล้วต้องทิ้งมันไป
• แค่นี้ก็อาจพอให้นักพัฒนายอมแพ้แล้ว

The Late-Breaking Design Review

• มีงานที่ซับซ้อนมากกำลังดำเนินมาเป็นเวลาหลายสัปดาห์หรือหลายเดือน ผ่านแพตช์ย่อยแยกกันจำนวนมาก
• ผู้รีวิวไม่เห็นด้วยกับการออกแบบของงานทั้งหมดนั้น แต่ไม่ได้เข้าร่วมการพูดคุยตั้งแต่แรก หรือเคยอยู่ฝั่งที่แพ้ในการถกเถียง
• แล้วตอนนี้กลับมีคำขอให้ผู้รีวิวช่วยดูส่วนเล็กๆ แต่สำคัญซึ่งอยู่กลางงานนั้น (เช่น การแก้บั๊กง่ายๆ ที่กำลังกีดขวางนักพัฒนาหลายคน) นี่คือโอกาสของผู้รีวิว
• ผู้รีวิวจึงเรียกร้องให้มีการให้เหตุผลต่อการออกแบบของงานทั้งหมดที่ทำมาจนถึงตอนนี้
• ถ้านักพัฒนารับผิดชอบเพียงส่วนหนึ่งของงานทั้งหมดและไม่รู้คำตอบ นั่นไม่ใช่ปัญหาของผู้รีวิว ผู้รีวิวจะไม่กดปุ่ม "Approve" จนกว่าจะพอใจ

The Catch-22

• ถ้าเป็นแพตช์ใหญ่ชิ้นเดียว ก็จะบอกว่าอ่านยากเกินไป และเรียกร้องให้แยกเป็นชิ้นย่อยที่ Self-Contained
• แต่ถ้ามีแพตช์เล็กหลายชิ้น ก็จะบอกว่าบางชิ้นไม่มีความหมายในตัวเอง แล้วเรียกร้องให้เอากลับมารวมกันใหม่
• ถ้าดูเหมือนว่ามี trade-off แบบใดแบบหนึ่งเกี่ยวข้องกับกรณีนั้น ก็สามารถฉวยใช้มันเป็นเหตุผลในการบ่นได้
  • ตัวอย่างเช่น ถ้าโค้ดเขียนให้อ่านเข้าใจง่าย ก็จะหาว่าประสิทธิภาพไม่ดี แต่ถ้า optimize มาดี ก็จะหาว่าอ่านยากและบำรุงรักษายาก

The Flip Flop

• มีการเปลี่ยนแปลงบางประเภทที่คนมักทำกับส่วนเดิมของโค้ดอยู่เป็นประจำ
• ผู้รีวิวเคยรีวิวการเปลี่ยนแปลงลักษณะนี้มาหลายครั้งแล้ว
• มีการเปลี่ยนแปลงใหม่เข้ามาอีก ผู้เขียนไปศึกษาการเปลี่ยนแปลงก่อนหน้าอย่างละเอียด ทำตามแพตเทิร์นเดิมอย่างระมัดระวัง และเลือกผู้รีวิวที่ดูคุ้นเคยกับบริเวณนี้
• แต่ผู้รีวิวกลับคัดค้านบางแง่มุมของการเปลี่ยนแปลงนั้นอย่างกะทันหัน ทั้งที่ก่อนหน้านี้ไม่เคยมีปัญหากับมันเลย การทำตามแพตเทิร์นเดิมจึงไม่พอ
• แล้วถ้าผู้เขียนชี้ให้เห็นความไม่สม่ำเสมอของผู้รีวิว โดยยกการเปลี่ยนแปลงแบบเดียวกันในอดีตขึ้นมาล่ะ?
  • ผู้รีวิวก็จะตอบว่า "จริงด้วย อันนั้นก็ควรถูกเปลี่ยนเหมือนกัน"
  • ผู้รีวิวต้องระวังอย่าอาสาไปแก้กรณีเดิมทั้งหมดเอง ถ้าโชคดี นักพัฒนาอาจตีความว่านี่คือคำสั่งให้ไปแก้กรณีเก่าด้วยตัวเอง ซึ่งช่วยประหยัดแรงผู้รีวิวได้มาก

แต่เอาจริงนะ ...

• ที่ผ่านมาทั้งหมดในบทความนี้เป็นเรื่องล้อเลียน ไม่ได้ต้องการเสนอว่าผู้รีวิวจงใจทำพฤติกรรมแย่ๆ แบบนี้
  • คำอธิบายส่วนใหญ่เป็นการพูดเกินจริงจากสิ่งที่ผู้รีวิวทำจริง หรือพูดเกินจริงจากสิ่งที่ผู้ส่งแพตช์ที่กำลังหงุดหงิดรู้สึก
• ในโลกความจริง อย่าทำแบบนี้!
  • พยายามลดจำนวนรอบไปกลับให้เหลือน้อยที่สุด ขอให้มีการเขียนใหม่ครั้งใหญ่ก่อน (ถ้าจำเป็น) แทนที่จะไปจู้จี้กับเรื่องเล็กน้อยก่อน และเมื่อวิจารณ์แพตช์ก็ควรเสนอแนะอย่างสร้างสรรค์ว่าคุณจะยอมรับเวอร์ชันแบบไหน
  • หากคุณมีความเห็นต่อ codebase โดยรวม ก็ควรคุยในภาพรวมกับนักพัฒนาทุกคน แทนที่จะมาใช้การรีวิวแพตช์ของนักพัฒนาคนหนึ่งเป็นที่จับผิด
  • ถ้าคุณเผลอทำแบบนี้ไปโดยไม่ตั้งใจ ก็ต้องตระหนักรู้ ยอมรับว่าคุณทำให้ชีวิตนักพัฒนายากขึ้นโดยไม่ได้ตั้งใจ ขอโทษ และพยายามช่วยให้มากขึ้น
• ปัญหารากฐานคือการใช้อำนาจในทางที่ผิด
  • เมื่อนักพัฒนาคนหนึ่งกลายเป็นผู้รีวิวโค้ดของแพตช์จากนักพัฒนาอีกคน เขาจะมีอำนาจชั่วคราวขึ้นมา ผู้รีวิวมีอำนาจที่จะขัดขวางไม่ให้แพตช์นั้นถูกคอมมิต
  • อำนาจมาพร้อมความรับผิดชอบ ควรใช้อำนาจเพื่อจุดประสงค์ที่ตั้งใจไว้ และใช้เท่าที่จำเป็น
  • แอนติแพตเทิร์นส่วนใหญ่ (หรือพฤติกรรมที่ไม่ร้ายแรงแต่ถูกเสียดสีไว้) คือการใช้อำนาจในทางที่ผิด ผู้รีวิวใช้ความได้เปรียบจากอำนาจชั่วคราวเหนือผู้พัฒนาเพื่อผลักดันวาระส่วนตัวที่ไม่เกี่ยวข้องหรือขัดกับการปรับปรุงแพตช์
  • วาระส่วนตัวอาจต่างกันไปตามแต่ละแอนติแพตเทิร์น เช่น งานที่ไม่เกี่ยวข้อง ความชอบด้านสไตล์ส่วนตัว ความขี้เกียจ การต่อต้านการเปลี่ยนแปลง หรือความไม่ชอบผู้ส่งแพตช์เป็นการส่วนตัว
• ถ้าผู้ส่งแพตช์เคยเป็นผู้รีวิวโค้ดแล้วทำพฤติกรรมแย่ๆ แบบนี้มาก่อน ความไม่ชอบนั้นก็อาจมีเหตุผลได้ ดังนั้นควรใช้อำนาจของผู้รีวิวโค้ดอย่างระมัดระวัง
  • ถ้านักพัฒนาต่างฝ่ายต่างเข้าสู่วงจรอุบาทว์ของการแก้แค้นกัน ซอฟต์แวร์โปรเจ็กต์ก็จะพังพินาศ

การรีวิวโค้ดแบบ Gatekeeping

• จนถึงตรงนี้ เนื้อหามุ่งไปที่การรีวิวโค้ดระหว่างเพื่อนร่วมงานเป็นหลัก
  • ผู้รีวิวโค้ดกับผู้ส่งแพตช์เป็นเพื่อนร่วมงานกัน ไม่ได้มีหน้าที่บังคับบัญชากัน หรือมีอำนาจตัดสินขั้นสุดท้ายเหนือ codebase
  • เพราะฉะนั้น อำนาจที่ได้จากการรีวิวโค้ดจึงเป็นเพียงชั่วคราว
• ในสถานการณ์แบบ peer review ผู้รีวิวโค้ดกับผู้เขียนควรมีเป้าหมายเดียวกันเป็นพื้นฐาน
  • หากมีความเห็นไม่ลงรอยกันอย่างจริงจังว่า ควรใส่ฟีเจอร์อะไร ต้องขัดเกลาแค่ไหนก่อนอนุมัติ หรือ coding style แบบใดที่ยอมรับได้ ก็ควรไปจัดการกันนอกบริบทของการรีวิวโค้ด
• แต่ในสถานการณ์การรีวิวโค้ดแบบอื่น เรื่องนี้ไม่จริงเสมอไป โดยเฉพาะเมื่อคนนอกโปรเจ็กต์ส่งแพตช์ที่ไม่มีใครร้องขอเข้ามา
  • สถานการณ์แบบนี้มักเกิดในวงการฟรีซอฟต์แวร์
  • เพราะใครก็ตามจากทั่วโลกสามารถแก้ซอร์สโค้ดได้ และบางคนก็พยายามส่งการเปลี่ยนแปลงกลับมา
• แต่มันก็อาจเกิดขึ้นในบริบทอื่นได้เช่นกัน
  • ภายในบริษัทที่พัฒนาโค้ดแบบ Proprietary นักพัฒนาจากทีมหนึ่งอาจส่งแพตช์ไปยังโปรเจ็กต์ของอีกทีมแล้วหวังว่าจะโชคดี
• ในสถานการณ์เช่นนี้ มีโอกาสมากกว่ามากที่ผู้รับแพตช์จะไม่ต้องการการเปลี่ยนแปลงนั้นตั้งแต่แรก หรือไม่เห็นด้วยอย่างสิ้นเชิงกับวิธีที่มันถูกทำ จึงอาจไม่รับแพตช์เลย
  • ในกรณีนี้ มันไม่ใช่การใช้อำนาจชั่วคราวที่ได้มาในฐานะเพื่อนร่วมงานอย่างไม่เหมาะสม แต่เป็นการใช้อำนาจถาวรอย่างชอบธรรมในฐานะผู้รับผิดชอบโปรเจ็กต์
  • ทิศทางของโปรเจ็กต์ซอฟต์แวร์ของฉัน ฉันเป็นคนตัดสินเอง
• เมื่อผู้รีวิวโค้ดทำหน้าที่เป็น 'gatekeeper' แบบนี้ การวิจารณ์แพตช์เพราะมันละเมิดหลักการออกแบบหรือข้อกำหนดทั่วไปที่มีอยู่ จึงไม่ใช่เรื่องผิดเสมอไป
  • อาจเป็นไปได้ว่าฉันเองก็ไม่รู้ว่าจะจะแก้ปัญหานั้นให้สอดคล้องกับข้อกำหนดได้อย่างไร
  • ที่จริง นั่นอาจเป็นส่วนที่ยาก และอาจเป็นเหตุผลเดียวที่ฉันยังไม่ได้ทำการเปลี่ยนแปลงแบบเดียวกันนี้เอง
• อย่างไรก็ตาม แม้ในบริบทของ gatekeeping การใช้ 'The Guessing Game' แบบไม่อธิบายอะไรเลยก็ยังเป็นการเสียมารยาท
  • โดยทั่วไปฉันพยายามอธิบายว่านักพัฒนาอาจมองข้ามส่วนที่ยากไป และถ้าฉันเองก็ไม่รู้คำตอบ ก็จะบอกตรงๆ ว่าไม่รู้
• แน่นอนว่าไม่มีข้อแก้ตัวสำหรับการถ่วงเวลาเชิงรับเชิงรุกอย่าง 'The Death of a Thousand Round Trips'
  • ถ้าคุณไม่ต้องการเอาแพตช์นั้นเข้าโค้ดจริงๆ เลย และคุณก็มีอำนาจโดยชอบธรรมในบทบาท gatekeeper ที่จะตัดสินแบบนั้น คุณสามารถบอกออกมาตรงๆ เพื่อไม่ให้ผู้ส่งต้องเสียเวลาเปล่าอีก

Disclaimer

• ตลอดหลายปีที่ผ่านมา ฉันได้สะสมบันทึกสำหรับบทความนี้จากการรีวิวโค้ดที่ฉันมีส่วนร่วมเอง (ทั้งสองฝั่ง) การรีวิวโค้ดที่ฉันสังเกตเห็นระหว่างคนอื่น และการรีวิวโค้ดที่ฉันเพียงได้ยินจากบทสนทนา
• ดังนั้นนี่ไม่ได้เจาะจงไปที่ใครคนใดคนหนึ่งที่เพิ่งมารีวิวโค้ดของฉัน

[intajon]

อาจจะไม่ได้พูดเกินจริงอย่างที่คิดก็ได้....

[magulhalmom]

อันนี้ผมเจอมากับตัวจริง ๆ จนเกือบเลิกเป็นนักพัฒนาไปเลย ตอนกลับมาตั้งหลักใหม่ก็ลำบากมากจริง ๆ

[bungker]

อ่านแล้วแทบ PTSD กำเริบ

[kayws426]

ดูเหมือนว่าคุณจะคอยรวบรวมบันทึกสำหรับบทความนี้มาได้ดีตลอดเลยนะ!!

[gongsil1212]

แค่อ่านก็เหมือนถูกทำร้ายทางจิตใจแล้ว...

[wedding]

สรุปว่าใจความสำคัญอยู่ที่บรรทัดสุดท้ายสินะ 555.....

[jypark]

ว้าว.. เกือบเป็นมะเร็งไปแล้วนะเนี่ย;;

[ahwjdekf]

เรื่องแบบนี้ ถ้าไปดูที่ที่ทำ si + sm ก็จะเห็นได้บ่อยในเกาหลีเหมือนกัน มักจะเรียกกันว่าเขตอิทธิพลเจ้าถิ่นนั่นแหละ พวกคนไม่ดีทำสารพัดอย่างเพื่อปกป้องชามข้าวของตัวเอง

[kenuheo]

มีวิธีที่ชั่วร้ายอยู่มากจริง ๆ

[gooksangom]

มองในระยะยาว คนที่ทำอะไรแบบนั้นโดยไม่มีเหตุผลที่สมเหตุสมผล สุดท้ายก็จะ 1) ถูกกันออกจากเครือข่ายคอนเนกชันของนักพัฒนาตั้งแต่เนิ่นๆ หรือ 2) ถึงจะนิสัยแย่แต่ความสามารถโดดเด่นมากจนรับผิดชอบงานก้อนใหญ่และตัดออกได้ยาก ก็จะมีใครสักคนทำหน้าที่เป็นตัวกลาง คอยเป็นช่องทางสื่อสารและประคองความเชื่อมโยงนั้นไว้ สภาพแบบนั้นถึงดำรงอยู่ได้ ดังนั้นถ้าคนกลางคนนั้นหายไปไม่ว่าด้วยเหตุผลใด ก็อีกไม่นานก็จะถูกตัดออกอยู่ดี ต่อให้เก่งแค่ไหน สุดท้ายงานต่างๆ ก็ต้องอาศัยคนมารวมตัวกันทำถึงจะมีเงินหมุนเวียน และเมื่อมีเงินหมุนเวียน คนก็ยิ่งต้องพึ่งพากัน เพราะฉะนั้นคนที่เข้ากับคนอื่นไม่ได้ก็มักจะถูกกันออกจากกลุ่มและค่อยๆ ถูกคัดทิ้ง

โดยปกติ คนที่นิสัยแย่แต่ยังอยู่รอดในกลุ่มได้นาน มักเข้าใจผิดว่าตัวเองอยู่รอดมาได้เพราะเป็นอะไรสักอย่างที่พิเศษยิ่งใหญ่ราวกับ Sherlock ในละคร เป็นพวก high-functioning sociopath อะไรทำนองนั้น แต่ความจริงก็แค่คนรอบข้างอดทนใช้ประโยชน์จากเขาเพราะยังมีคุณค่าให้ใช้งาน พอหมดประโยชน์เมื่อไร ความสัมพันธ์ก็จะกลายเป็นประมาณว่า "อยู่ด้วยกันแล้วสกปรกใจ อย่าได้เจอกันอีกเลย" แม้แต่ Sherlock เวอร์ชันที่ Benedict Cumberbatch แสดงนำ ก็เป็นเพียง sociopath ที่เรามองจากภายนอกแล้วรู้สึกว่ามีเสน่ห์เท่านั้น ถ้ารอบตัวเขาไม่มีคนที่ไม่ยอมทอดทิ้งและยังคอยเห็นคุณค่าเขาอยู่ เรื่องราวก็คงไปต่อไม่ได้เลย.

[dokdok]

คนที่นิสัยแย่แต่ยังอยู่รอดมาได้นาน มักเข้าใจผิดว่าที่ตัวเองอยู่รอดได้เป็นเพราะตัวเองเป็นอะไรที่ยิ่งใหญ่ ราวกับเชอร์ล็อกในละคร เป็นพวก sociopath ประสิทธิภาพสูงอะไรทำนองนั้น แต่จริง ๆ แล้วก็แค่คนรอบตัวอดทนใช้ประโยชน์จากเขาเพราะยังมีคุณค่าให้ใช้เท่านั้น พอคุณค่านั้นหมดไป ความสัมพันธ์ก็จะกลายเป็นแบบ "ร่วมงานกันแล้วมีแต่ความสกปรกใจ อย่าได้เจอกันอีกเลย" ==> เป็นประโยคที่คมมากครับ/ค่ะ ต้องจำไว้เลย

[savvykang]

ทำให้นึกถึงการกลั่นแกล้งในที่ทำงานหรือไม่ก็ power harassment เลยครับ

[xguru]

บอกว่าเป็นเรื่องขำ ๆ แต่พออ่านไปแล้วหงุดหงิดขึ้นมาปรี๊ดเลย..