DRY Right way part 3: Reduplicating code

เอาล่ะ หลังจากจบ Part 1 กับ Part 2 มาแล้วเราก็จะเริ่มดู Practical guide ที่จะแก้ไขปัญหาแล้ว

แต่ก่อนอื่นผมขอทบทวนสิ่งที่อยากสื่อใน Part 1 หรือ 2 ซ้ำอีกรอบนะครับ ว่าให้เราระวังเรื่องการสร้าง Abstraction ขึ้นมาเร็วเกินไปให้ดี บางครั้งยอมปล่อยให้โค้ดมี Duplication อาจจะดีกว่าสร้าง Abstraction ที่แปลกประหลาดและเข้าใจยาก

สำหรับ Code metric การวัดว่าโค้ดดีหรือไม่ดี มันจะมีทั้งเรื่อง Redundancy (ความซ้ำ) ที่ต้องมีน้อย และ Dependency (ความถูกต้องของโค้ดขึ้นอยู่กับ Class / Function อื่นๆ)

การสร้าง Abstraction นั้นจะลด Redundancy แต่เพิ่ม Dependency ขึ้นมา ดังนั้นก็คิดให้ดีก่อนว่าเราอยากจะสร้าง Dependency แบบไหนถึงจะคุ้มกับ Redundancy ที่ลดไป

เรื่องนี้ผมอยากสื่อแต่ออกจะเวิ่นเว้อไปมากในสอง Part แรก เลยขอทวนสั้นๆ อีกครั้ง

โอเค เอาล่ะ ปัญหาคือบางครั้งใน Legacy code เราก็มี Wrong abstraction ไปเรียบร้อยแล้ว ประเภท Class ที่ต้องใช้ IF เยอะแยะเต็มไปหมด แน่นอนเราก็ไม่ควรเพิ่ม IF เข้าไปอีกชุดแน่นอน แล้วคำถามคือเราจะแก้ไขอย่างไรดีล่ะ

ขั้นตอนที่ผมชอบใช้จะเป็นแบบนี้ครับ

1. Re-duplicate code ขึ้นมาใหม่หมด ดูซะว่าถ้าเราถอดส่วนโค้ดที่แชร์กันออกมาเป็นส่วนๆ ไม่ให้มันใช้โค้ดร่วมกันเลย มันจะหน้าตาเป็นอย่างไร
2. ดูส่วนที่มันซ้ำและแตกต่างกัน
3. สร้าง Abstraction ตามส่วนซ้ำและส่วนต่างที่เกิดขึ้นหลังจากกางโค้ดออกมาดูหมดแล้ว

พูดแบบนี้อาจจะงง ผมขอยกตัวอย่างจาก Part 2 เลยละกันนะครับ

Requirement ของระบบนี้คือ

1. สามารถเก็บใบเสร็จได้ โดยแต่ละใบเสร็จจะประกอบด้วย ชื่อรายการ (Description) จำนวนรายการ (Quantity) และราคาต่อหน่วย (Price per unit)
2. สามารถคำนวนราคาได้โดยเงื่อนไขดังนนี้

• ถ้าเป็นใบเสร็จธรรมดา ให้เอา ราคาต่อหน่วยคูณกับจำนวนรายการได้ จะเป็นราคาใบเสร็จก่อนภาษี
• สำหรับใบเสร็จของแผนก X เขาซื้อขายงานโปรเจ๊กต์ ดังนั้นราคาที่อยู่ในต่อหน่วยจะเป็นราคารวมอยู่แล้ว ไม่ต้องสนใจคูณจำนวน
• สำหรับใบเสร็จของแผนก Y เขาจะจ่ายมัดจำ 20% ก่อนเสมอ ดังนั้นให้หักราคามัดจำทิ้งได้เลยเวลาคำนวณ

3. หลังจากได้ราคาแล้วให้มันคำนวณภาษีต่อ

• สามารถกำหนดได้ว่าใบเสร็จนี้มีภาษี VAT มั้ย ถ้าไม่มีก็ไม่ต้องเพิ่ม
• แต่ใบเสร็จที่ไม่มี VAT อาจจะมีภาษีพิเศษด้วยได้
• ภาษีพิเศษจะเป็นภาษีหักเพิ่มเติม 5%
• ใบเสร็จแผนก X จะมี VAT เสมอ แผนกอื่นจะไม่มี

Legacy code ถูกเขียนไว้ตามที่ระบุใน Invoice.cs

เอาล่ะ เรามาแก้ไข Legacy code กันเถอะ

สิ่งที่ผมเริ่มทำคือผมจะพยายามกำจัด IF ที่เยอะๆ นี้ออก

คำถามแรกคือ ถ้าสมมติเราคิดซะว่า isDepartmentX เป็น true เสมอ คือเราจะสร้างสำหรับรองรับ แผนก X เท่านั้น หน้าตาของ Class จะเป็นอย่างไร

ผมก็ Copy-paste Code Invoice.cs ไปเป็น InvoiceDepartmentX.cs แทน แล้ววิเคราะห์ว่าโค้ดบรรทัดไหนบ้างที่ถ้าเราคิดว่ามันเป็น แผนก X เสมอแล้ว มันไม่ต้องมีก็ได้

ผมสรุปการวิเคราะห์ไว้ประมาณนี้

แล้วเราล้างส่วนที่ไม่จำเป็นออกทั้งหมด ก็จะได้ Class ใหม่ตามนี้

โอ้ว สั้นลงและเข้าใจง่ายขึ้นเยอะเลยนะเนี่ยยยย

แน่นอนว่าตอนนี้เราจะมีโค้ดซ้ำระหว่าง Invoice.cs กับ InvoiceDepartmentX.cs แต่ไม่เป็นไร เราปล่อยซ้ำไปก่อนได้

ผมทำแบบเดียวกับ DepartmentY จะได้การวิเคราะห์ออกมาแบบนี้

แล้วพอล้าง Cleanup ส่วนที่ไม่ต้องใช้ทั้งหมด จะได้ Class InvoiceDepartmentY หน้าตาดังนี้

เห้ย InvoiceDepartmentY สะอาดขึ้นตั้งเยอะนี่นา

เอาล่ะ ผมทำแบบเดิมกับ InvoiceNormalCase.cs ได้ผลดังนี้ (ขอข้ามส่วนวิเคราะห์นะครับรอบนี้)

พอเรากางโค้ดออกมาทั้งหมด เราถึงจะเห็น Pattern จุดเหมือนและจุดต่างของ Method CalculateSumPrice() ของ Invoice ทั้งสามประเภทได้อย่างชัดเจนขึ้น แต่ละคลาสตอนนี้ซ้ำกันเต็มไปหมดเลย แต่อ่านง่ายขึ้นมากๆ

กาง 3 Class มาเทียบ Duplication กัน

จุดเหมือน

• ทั้งสามแบบ การคำนวณราคาสุดท้าย ต้องคำนวณ calculateSumPrice แล้วค่อยคำนวณภาษี แล้วจึงตอบออกไปทั้งสาม Class

จุดต่าง

• การคำนวณ SumPrice ของ Invoice แต่ละแบบนั้นไม่เหมือนกัน
• การคำนวณภาษี VAT สุดท้ายนั้นไม่เหมือนกัน

ถ้าอิงหลัก OOP เราอยากจะไม่สนใจว่าด้าน Implementation อย่างไร เราสนใจแค่คุณไปคิดราคา กับคิดภาษีมาซะ ส่วนคิดยังไงค่อยไปว่ากันทีหลัง เราจะอยากเขียนโค้ดส่วนที่เหมือนกันออกมาแบบนี้

เราอยากให้โค้ดสุดท้ายออกมาหน้าตาแบบนี้ให้ได้ คือสนใจแต่ Interface แต่ไม่สนใจ Implementation ที่อาจจะแตกต่าง

ซึ่งมันจะอ่านง่ายมากๆ อ่านแค่ว่า calculateSumprice ให้คิดผลรวมนะ ให้คิดภาษีนะ จบ ไม่มี IF อะไรมากวนใจมากมายมหาศาลจนเวิ่นเว้อ

ถามว่าเราจะทำให้โค้ดสุดท้ายหน้าตาแบบนี้ได้อย่างไร ผมคิดออก 2 ทางเลือก

1. ใช้ Inheritance มองว่า Invoice ทุกใบมันต้องคำนวณ SumPrice ได้เหมือนกัน แล้วสร้าง Base-Invoice ขึ้นมาตัวนึงโดยไม่ Implement calculateSumPrice และไม่ Implement plusVat ให้ Sub-class แต่ละตัวไป Implement กันเอาเองตามว่าตัวคุณเป็นของ DepartmentX หรือ DepartmentY
2. ใช้ Composition โดยมองว่า Invoice ประกอบด้วย ส่วนการคำนวณภาษี ส่วนการคำนวณราคารวม แล้ว Inject เครื่องคำนวณผลรวม (SumPriceCalculator) เป็นอีกคลาสนึง ที่ถูกต้องตามแต่ละเคสเข้ามา

ทั้งสองท่าผมพบว่าจริงๆ ไม่ได้แตกต่างกันมากและค่อนข้าง Clean ทั้งคู่ แต่ผมชอบ Composition ดังนั้นผมจะอธิบาย Though process ของการใช้ Composition ในบทความนี้นะครับ

อันนี้สำคัญมาก คือ ผมเชื่อว่า เราไม่ควร Dogma ว่าโค้ดต้องเป็น “แบบนี้” ถึงจะถูกต้องที่สุด ไม่ใช่ว่า Inheritance เท่านั้นถึงจะดีกว่า Composition เท่านั้นถึงจะดีกว่า เราควรจะเรียนรู้ท่าต่างๆ ไว้ แต่ไม่จำเป็นต้องไปยึดติดว่า “ถ้าไม่ใช่วิธีการแบบนี้โค้ดคุณไม่ได้เรื่อง”

ในกรณีนี้โค้ดเก่าเราบอกได้ว่าโค้ดมันอ่านยากก็จริง เพราะมันมีหลาย IF หลายเงื่อนไข และเราพอจะบอกเป็นความรู้ทั่วไปได้ว่า IF ซ้อนเยอะๆ คืออ่านยาก แต่ถ้าเราแกะ IF ออกมา ไม่ว่าจะด้วยท่าไหนก็ตาม ผมคิดว่าแต่ละท่ามันมี Tradeoff ของมันอยู่ที่แตกต่างกัน แล้วเราไม่ควรจะยึดติดว่า ท่านี้เท่านั้นถึงจะเวิร์ค อีกท่าคือไม่ดี

ปกติการเลือกว่าจะใช้ Inheritance หรือ Composition ในแง่นีถ้าผมมองว่าทางเลือกสองทาง ไม่ได้มีผลให้ความ Clean ของโค้ดแตกต่างกันอย่างมีนัยสำคัญ ผมจะให้น้ำหนักกับท่าที่คนในทีมคุ้นเคยเป็นหลักครับ

ผมเชื่อว่า Architect ที่ดีคือเราทำให้ทีมของเราแก้ไขง่าย Move fast ไม่ใช่แค่ใช้ท่าที่ถูกต้องตามหลักการ Software architecture

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

แต่ในโจทย์นี้เราไม่มีทีม ดังนั้น ผมเอาท่าที่ชอบส่วนตัวเลย ใช้ Composition ละกัน

โอเค ก่อนอื่น ผมจะเขียนปลายทางของ Class Invoice ที่ผมอยากให้เป็นก่อนนะครับ โดยเอาส่วนที่ซ้ำกัน เขียนจะได้หน้าตาแบบนี้

เห้ย ดูสวยงามขึ้นมาเลยอ่ะ เหลือแค่ 26 บรรทัดแล้วทุกอย่างเคลียร์มากว่าคลาสนี้ ฉันแค่คำนวนราคารวม แล้วฉันก็คำนวนภาษี จบ ไม่มี IF ทำโน่นทำนี่ในคลาสนี้ให้วุ่นวายใจแล้ว

แต่แน่นอน sumpriceCalculator นั้นอาจจะมีวิธีคำนวนที่ “แตกต่างกัน” ขึ้นอยู่กับว่าคนใส่เครื่องคิด Sumprice เข้ามาเป็น Class อะไร

จำได้มั้ยครับ ตอนที่เราบอกจุดเหมือนกับจุดต่าง เราบอกว่าจุดเหมือนคือ Flow เหมือนกัน แต่วิธีคิดต่างกัน

นี่แหละ ใช่เลย เรารวมจุดเหมือนไว้เป็น Flow ง่ายๆ โดยไม่ต้องมี IF ให้มันซับซ้อนจนสับสน แต่แยกจุดต่างออกไปเป็นอีกคลาสที่ใส่เข้ามา

นี่แหละคือพลังของ Polymorphism ทำให้อ่านง่ายและเคลียร์ว่าพยายามทำอะไร อันนี้อ่านปราดเดียวก็รู้แล้วว่าคลาสนี้ คำนวณราคานะ คำนวณภาษีต่อนะ คืนค่านะ จบ เคลียร์มากๆ

หลังจากนี้เรามาสร้าง SumpriceCalculator สำหรับเคสต่างๆ แล้ว Inject มันเข้ามาด้วย Factory Pattern กันดีกว่าครับ

ถ้าอ่านง่ายๆ คือ

1. เราแยกเครื่องคำนวณราคาออกเป็น 3 Class สำหรับ แผนก X แผนก Y และแผนกทั่วไป แล้วแต่ละเครื่องมีวิธีคิดไม่เหมือนกัน
2. ให้คนเลือกเครื่องคำนวณราคารวมเป็น SumpriceCalculatorFactory

ถึงตรงนี้ยังมี Duplication อยู่บ้างตรงที่ Normal กับ DepartmentY ตัว plusVat ยังเขียนซ้ำกัน แต่ผมมองว่าผมยังไม่ควรให้มันใช้ที่เดียวกัน ยังรู้สึกว่า Duplication แค่นี้พอจัดการได้อยู่มันไม่ได้เยอะจนควรจะรีบสร้าง Abstraction ขึ้นมาครอบ (ซึ่งตรงนี้บางท่านอาจจะเห็นต่างกับผมได้นะ)

ผมอยากสื่อว่า Knowledge ว่าการคำนวณภาษีมันไม่เหมือนกันเลยในแต่ละแผนก เลยยังไม่อยากรีบทำ Abstract เป็น Class ใหม่อันเดียว เพราะยังงั้นมันจะแปลว่าผมสื่อลงโค้ดว่า “ภาษีแผนก Y กับ ภาษีแผนกทั่วไป ใช้วิธีคิดเหมือนกันนะ” ซึ่งผมไม่แน่ใจว่ามันจะเป็นยังงั้นมั้ย หรือจริงๆ วิธีคิดภาษีมันต่างกันคนรับผิดชอบคนละคนกัน แต่แค่บังเอิญปีภาษีนี้ดันคิดเหมือนกัน อันนี้ก็อาจจะต้องถามฝั่ง Business อีกที

แต่เอาเป็นว่าผมปล่อยไว้ละกันครับ ฮา อันนี้ขอแถไปละกันเพราะโจทย์ไม่เคลียร์ ทำแบบไหนก็ไม่ได้แตกต่างกันมากจนเห็นได้ชัด

พอถึงตรงนี้ เวลาใช้ Invoice หลังจากนี้เราก็ไม่ต้องสร้าง Flag เยอะๆ แล้วเราใช้แค่นี้

ตัวคลาส Invoice ก็จะคำนวณราคารวมที่ถูกต้องให้โดยอัตโนมัติ ขึ้นอยู่กับเครื่องคำนวณที่ Inject เข้าไป ส่วนเครื่องคำนวณเราเลือกได้ว่าจะใช้เครื่องคำนวณสำหรับแผนก X หรือแผนก Y หรือแผนกทั่วไป

หรือจะสร้าง InvoiceFactory ขึ้นมาเลยก็ได้นะ

(หลังจากเขียนมาถึงตรงนี้ผมพึ่งสังเกตว่า Invoice ตัวต้นฉบับแอบมี Dead-code อยู่ด้วยตรงที่ไม่มีทางเข้า price * 1.07 * 1.03 แฮะ)

เทียบของเก่ากับใหม่

ซึ่งถ้าเทียบกับโค้ดเดิมที่เป็น Invoice ตัวต้นฉบับแล้วจะพบว่า

1. Invoice.cs เราสั้นและเคลียร์ขึ้นอย่างเห็นได้ชัด อันนี้อ่านง่ายมากเลยว่าคลาสนี้มีหน้าที่แค่ดำเนิน Flow การทำงาน คิดผลรวม คิดภาษี จบ
2. จุดที่แตกต่างคำนวณ 3 แบบ ถูก Abstract ไปไว้คลาสอื่นโดยมี Factory เป็นตัวเลือก ทำให้เวลาแก้ไขโค้ดถ้ามีคนบอกว่า “แก้ไขวิธีคำนวณของแผนกทั่วไปหน่อย” เราก็แก้ไขได้โดยไม่ต้องหานาน แค่ไปแก้ที่ NormalCalculator ได้เลย มั่นใจได้ด้วยว่าไม่กระทบกับ DepartmentXCalculator เพราะเป็นคนละคลาสกัน ถูกแยกกันอย่างชัดเจน ไม่พันกันอยู่ใน IF-Clause ยาวๆ ซับซ้อนๆ แบบต้นฉบับ

การที่เราแยกโค้ดที่แตกต่างออกมาเป็น Class ที่ Interface เหมือนกัน ทำให้โค้ดส่วนที่เหมือนอยู่ด้วยกันอย่างแนบแน่น (Invoice.cs) ที่เหมือนกันหมด และส่วนที่แตกต่างกันก็ถูกแยกกันอย่างชัดเจน (SumpriceCalculator 3 Class ที่ Implement Interface แบบเดียวกัน)

ตรงนี้แหละที่ผมคิดว่ามันอ่านง่ายขึ้นมากและเป็นระเบียบขึ้นมาก รวมจุดเหมือนเป็น Code flow เดียวกัน และแยกจุดต่างเป็นคนละคลาสที่ Interface เดียวกัน

Key takeaway

จุดสำคัญที่ผมอยากจะสื่อในบทความนี้ หลายครั้งเราจะเห็นโค้ดแย่ๆ แล้วพยายาม Refactor ทันทีซึ่งเราก็จะงงและมึนพอสมควรว่า IF หรือ Subclass จำนวนมากมายแบบนั้นเราจะสามารถแก้ไขได้ยังไง

ผมพบว่าพอเราพยายามกางให้เกิด Duplicate code ขึ้นมาก่อน เราจะเห็น Pattern ตามธรรมชาติได้ง่ายขึ้นมาก

อย่างในตัวอย่างนี้ การกางออกมาสร้าง Full duplication ผมว่าง่ายกว่าการอ่าน Invoice.cs ที่เละเทะแล้วพยายาม Refactor ทันที

ไม่ใช่ว่าเราใช้ Sense แล้ว Refactor ทันทีไม่ได้นะครับ ผมก็ใช้ Sense อยู่บ่อยๆ และบางครั้งผมเองก็เห็น Code แล้วดูออกแต่แรกว่า Pattern มันไม่ดียังไง (แน่นอน ก็ตั้งโจทย์เองนี่)

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

แล้วเราจะ Refactor ได้ง่ายขึ้นมาก

อยากให้ลองอ่านอีกทีนึงว่าถ้าเราพยายาม Refactor จาก Invoice.cs ต้นฉบับเลย เทียบกับกางออกมาให้มัน Duplicate กันก่อนแบบที่ผมทำให้ดูใน Gist ที่ผมกางออกมาทั้งหมด ให้มันเกิดโค้ดซ้ำๆ เต็มไปหมดออกมาให้เต็มที่เลย แล้วค่อย Refactor ผมพบว่า การกางออกมาก่อนมักจะทำให้เราไม่ค่อยหลงทาง

ที่ผมเน้นว่า“duplication is far cheaper than the wrong abstraction” ก็เพราะแบบนี้แหละครับ

ผมเห็นว่าปกติแล้วการ Refactor โค้ดที่ซ้ำ มันง่ายกว่าพยายาม Refactor ของที่ Abstract ผิดมากๆ

ดังนั้นทริกนึงที่ผมใช้คือ “กลับไปทำให้มัน Duplicate กันก่อน”

ซึ่งผมพบว่ามันช่วยแก้ Wrong abstraction ได้ดีมากๆ ครับ

เสริมเพิ่มเติม

จริงๆ หลายๆ ท่านคงเคยอ่านเรื่อง TDD เราต้องทำ Red-green-refactor ใช่มั้ยครับ คือเราเขียนเทสก่อน ทำให้เทสผ่าน แล้วค่อย Refactor

ผมคิดว่ามันเกี่ยวข้องกับเรื่องนี้มาก คือ เราทำโง่ๆ ให้เทสผ่าน มี Duplication มีความเวิ่นเว้อ ให้มากที่สุดเท่าที่จะมากได้ก่อน แล้วค่อย Refactor มันทีหลัง

ผมพบว่ามันมักจะได้ Pattern ของ Abstraction ที่ดีกว่าการที่พยายาม Red-Greent+Refactor พร้อมๆ กันครับ การเขียนโง่ๆ ให้เทสผ่านไปก่อนจะทำให้เราเห็น Pattern ชัดเจนมาก

ซึ่งถ้ายังไม่ได้เก่ง Design pattern พอที่จะเห็นทุกอย่างในหัวแต่แรก ผมคิดว่า การกางโค้ดให้โง่ที่สุดก่อน Refactor เป็นอะไรที่ดีมากครับ มันง่ายกว่าเรารีบ Refactor ระหว่างทำให้ Test ผ่าน สร้าง Abstraction ผิด แล้วต้องมาแก้ไขใหม่ทีหลัง

ส่วนตัว ผมเองถ้าผมไม่มั่นใจมากๆ ใน Pattern ที่ใช้ ผมก็รอทำ Requirement ให้ผ่านทุกเทสก่อนค่อย Refactor เหมือนกันครับ ไม่ได้ทำ Refactor ระหว่างทำให้เทสผ่านีเท่าไหร่นัก

ส่งท้าย

ผมยังยืนยันว่า การโค้ดมันไม่มี “ท่าที่ดีที่สุด” ดังนั้น ใครมีวิธี Refactor โจทย์ตัวอย่างที่คิดว่าดีกว่าสวยกว่าของผมก็เสนอได้ใน Comment ครับ หรือมีความเห็นอย่างไรว่าตรงไหนที่มันดีหรือไม่ดี มี Trade-off อะไรบ้างก็แลกเปลี่ยนความรู้กันได้ครับ หรือแม้แต่แค่บอกว่า ไม่ชอบท่าผม อยากใช้ท่าอื่น ก็บอกได้ครับ (แค่ชอบไม่ชอบก็เป็นเหตุผลได้นะ เพราะการเขียนโค้ดบนท่าที่ชอบก็ Productivity ดีขึ้นนะ ถ้าท่าที่ชอบไม่ได้สร้างความปวดหัวมากเกินไป)

ผมเองก็ไม่ได้คิดว่าท่าที่ใช้นี้ดีที่สุดนะครับ แต่คิดว่าน่าพอใจในระดับนึงแล้วครับ