Basic code smell#1: Method ที่ขึ้นกับ Mutation

บ่อยครั้งมากๆ ที่ผมจะเจอปัญหาแบบนี้ ทั้งในโค้ดตัวเองและโค้ดของคนอื่น แล้วผมพบว่ากว่าผมจะเห็นว่ามันเป็น Code smell ก็ต้องสั่งสมประสบการณ์มาเยอะเหมือนกัน

สมมติว่าเราต้องการส่ง email ปกติเรามักจะเห็นโค้ดแบบนี้

var emailSender = new EmailSender()
emailSender.to = "ace@taskworld.com"
emailSender.body = "Hello ace"
emailSender.send()

โค้ดลักษณะนี้เห็นได้ทั่วไปมากๆ

คำถามคือแล้วถ้าเกิดว่าอยู่ดีๆ มีคนอุตริส่ง email.send() โดยที่ลืมใส่ email.to ล่ะ

ผมพบว่าปกติระบบก็จะฟ้อง Exception ว่าอย่าลืมใส่ว่าส่งอีเมล์ให้ใคร

ก็ฟังดูปกติเนอะ เราก็เคยชินกับระบบแบบนี้คลาสแบบนี้กัน ก็ยังดีกว่าอยู่ดีๆ เงียบไปเลยแล้วส่งเมล์ไม่ได้ หรือทำอะไรแปลกๆ อ่ะนะ

แต่ผมคิดว่ามันดีไม่พอ

สิ่งที่ดีกว่านั้นคือออกแบบให้โปรแกรมเมอร์ไม่สามารถเขียนโค้ดที่เกิด Runtime exception ได้ตั้งแต่แรกเลย!!!

“Making illegal states unrepresentable”

หา เราทำถึงขนาดนั้นได้ด้วยเหรอ?

ผมตอบว่าในกรณีนี้ทำได้ครับ

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

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

ในกรณีนี้ทำได้สองแบบครับ

Object-Oriented approach

var emailSender = new EmailSender("ace@taskworld.com", "Hello ace")
emailSender.send()

ในกรณีนี้ ผมออกแบบ Class EmailSender ไม่ให้มี Empty Constructor ดังนั้นไม่ว่าใครใช้ก็ต้องใส่ข้อมูลให้ครบก่อนเสมอ

พอเราบังคับให้ Constructor ต้องมีครบ ก็จะไม่มีใครสามารถสร้าง EmailSender ที่พอกด emailSender.send() แล้วจะเกิด Exception ได้อีกแล้ว

Functional Approach

send(emailServer, "ace@taskworld.com", "Hello ace")

ง่ายและตรงไปตรงมา ผมส่ง Function บอกว่าต้องใส่ข้อมูลให้ครบ แค่นี้ก็เป็นการยากมากแล้วที่ Developer จะสร้าง Runtime error ได้

นี่คือเหตุผลนึงที่ผมชอบ Functional programming เนื่องจากมันไม่มี State mutation มีแค่คำสั่งกับ Data transformation ดังนั้นเราสามารถบอกได้ทันทีว่าเราต้องการอะไรบ้าง ส่งมาให้ครบ เราไม่สามารถสร้าง Invalid state ได้แต่แรกแล้ว

Common Popular Anti-Pattern

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

(ผมถึงได้บอกทุกคนตลอด อย่าเชื่อว่านี่มันเป็น Best practice ออกแบบมาดี เพียงเพราะบริษัทใหญ่ๆ อย่าง F…., G…, M… หรือใครก็ตามออกแบบมาให้เรา ตั้งคำถามด้วยว่าเขาทำมาดีหรือเปล่า)

ORM

user = new User()
user.email = "ace@taskworld.com"
user.name = "Chris"
user.password = Hash("WeirdPassword")
user.optionalTelephone = "0000000"
user.save() // *** Throws if some required fields missing ***

นี่คือ Orm ทั่วไปที่พบได้ในเกือบๆ ทุกภาษา

ถ้าในกรณีนี้ user นั้นต้องการ email, name, password แน่นอนในการสร้าง (คงไม่มี User คนไหนที่ไม่มีอ่ะนะ)

แทนที่เราจะบอกว่า user.save() คุณไปทำ Validation ด้วยนะ ใครส่งข้อมูลมาไม่ครบให้ Throw runtime exception นะ จะดีกว่ามั้ยถ้าเราออกแบบยังงี้

user = new User("ace@taskworld.com", "Chris", Hash("WeirdPassword")
user.optionalTelephone = "0000000"
user.save()

ถ้าเราออกแบบยังงี้ ไม่ว่าเราจะกด user.save() ตอนไหน ไม่ว่า Programmer จะมาซนยังไง ตราบใดที่เขาไม่เปลี่ยน Interface ของ Class user ก็ไม่มีทางเกิด Runtime exception จากการใส่ฟิลด์ที่ต้องการไม่ครบได้เลย

นี่เป็นหลักการที่เข้าใจง่าย แต่ก็พลาดกันบ่อย แม้แต่ผมเองก็ใช้เวลาหลายปีกว่าจะมองเห็นหลักการง่ายๆ ตรงนี้

สิ่งที่ผมอยากให้จำคือ ครั้งถัดไปที่เราใช้ Library หรือใช้ Class ของคนอื่น หรือใช้ Function ของคนอื่น แล้วมัน Throw runtime exception ขึ้นมาเพราะเรา Config อะไรบางอย่างไม่ครบ

อย่ามองแต่แค่โทษตัวเองว่าเราโง่เองไม่ยอมอ่านให้ครบก่อนใช้เพียงอย่างเดียว พี่เขาก็บอกแล้วใน Doc แต่ไม่อ่านอย่างเดียว

แต่อยากให้มองให้เห็นไปถึงว่าการดีไซน์คลาสที่เกิดสิ่งนี้ได้ มันเป็น Code smell แบบนึงด้วยครับ