รอบนี้ยาวเลยมาหลังสงกรานต์ หลังจากเคลียร์งานธุระต่างๆหมด หัวข้อมี ดังนี้
What is Code Smells ?
- อะไรที่เราสามารถเอ๊ะ/แหม่งๆใน Code ได้ (Quick Spot) และเป็นจุดที่ทำให้เราต้องมาปรับปรุง หรือแก้ไข ก่อนที่ Code จุดนั้นทำให้เกิดปัญหาได้
Code Smells are symptoms of poor design or implementation choices
Martin Fowler
Type Of Code Smells
มีเยอะมาก แบ่งได้ 5 กลุ่ม
Bloaters - Don't have to be that big มันใหญ่ไป
- Primitive Obsession พวก Primitive มันใช้ if เยอะ หรือ Data Structure ที่ซับซ้อนมากไป ทำให้เป็น Class / Struct ดีกว่า
- Large Class
- Data Clumps - data ที่ต้องไปด้วยกันเสมอ แต่เราดันแยกส่ง แทนที่ทำให้เป็น Class
- Long Method - Code ยาวไปมีการทำงานหลายอย่างใน method เดียว
- Long Parameter List - 1 method รับหลาย param ตัวเลขหลายคนบอก 3 / 5 ผมใช้ 5 นะ
Tool Abusers - Misuse
- Alternative Classes with Different Interfaces - Method/Class ที่ทำหน้าที่เดียวกัน แต่ดันรับ Interfaces parameter คนละแบบ ไม่เป็น orthogonal design (อิสระจากกันชัดเจน)
- Refused Bequest - Class ลูก Override Class แม่หมด ไม่รับมรดก แสดงว่า Design ผิดแล้ว
- Switch Statements - ปัญหา อาจจะเป็น Default Case อาจจะทำให้เกิดเคสแปลกได้
- Temporary Field - มีตัวแปรที่ไม่จำเป็น มันกิน mem และจัดการยาก เป็นไปได้ลด ถ้าๆไม่ได้ตั้งชื่อให้เหมาะสม
Change Preventers -ปรับเปลี่ยนยาก //จริงๆ Maintain ยาก
- Parallel Inheritance Hierarchies
- เมื่อก่อนนิยามไปแนวๆนี้มั้งที่เรียนมา จำๆได้ว่า C++ มันทำ multiple Inheritance ทำได้ Extend Class แม่ A / B ไปๆมามัน Conflict กันเอง แล้วไป override แทน เรื่องตลกร้าย คนที่มาใช้ override อีก ตอนแก้เลยยาก
- ตอนนี้ when an inheritance tree depends on another inheritance tree by composition ผมวา่าคำนี้อธิบายดีสุด
Note go ไม่มี inherited มีแต่ composite - Shotgun Surgery - ทุกอย่างกระจายหมด พอแก้ Feature นึง ปรากฏว่า เราต้องมาไล่แก้หลายจุด แบบปืน ยิงแล้วไป กระสุนแตกกระจาย เหมือนทำ Feature แปะหลายที่ แก้ไม่หมด
- Divergent Change - Code ที่่เป็น ความหวังหมู่บ้าน กลับด้านของ Shotgun Surgery รวมศูนย์ Logic
Dispensable - ไม่จำเป็นต้องมี
- Comments - เขียน Code ให้สื่อแทน เช่น เอาส่วนของ Code แยกเป็น method
- Data Class - Class ที่เก็บ Data แต่จริงมันจะขัดกับบาง Pattern เช่น DTO
- Lazy Class - บางอันไม่จำเป็นต้องแตก Class เป็น Untility เล็กๆได้
- Dead Code - ไม่ได้ใช้
- Speculative Generality - Generic เขียนเผื่ออนาคตเกิดไป ทำให้คนเอาไปใช้ต้องมีท่าแปลกๆ
- Duplicate Code - Code ซ้ำ
Couplers - Too much depend on each other
- Feature Envy - Feature ไปมันไปกองที่ Class อื่นมากเกินไป เหมือนใช้เพื่อนทำงาน
- Inappropriate Intimacy - Access Private Method / Field ของ Class อื่นๆ มัน Couple มากเกินไป //ไปปรับ State ของเค้านะ
- Incomplete Library Class - Library ยังทำไม่เสร็จ ใช้ได้บางเรื่องที่เหลือ อาจจะทำงานผิด หรือโยน Exception ออกมา
- Message Chains - ส่ง Data ไปมา แล้ววนไปเรื่อย กว่าจะกลับมาทำเสร็จ //Ping-Pong meeting ที่น่าจะมีจุดจบ
- Middle Man - โยนไม่ๆ คำสวยๆเป็น Proxy ส่งต่องานให้ Class อื่นๆทำ ตัวมันเองไม่มี Logic เลย
จริงต้องไปดู Coupling Cohesion
เหมือน Recap ป โท เลย ลองดูจาก Lecture 2110624_SDD_01.00.00.pdf ได้ น่าจะทำมา 6-7 ปี และ
Refactoring
Improving design without changing behaviour
อันนี้สรุปสั้นๆ ชัดดีชอบ
หรือ Behavior-preserving transformation
- แบบยาวๆ — Martin Fowler
- (noun): a change made to the internal structure of software to make it easier to understand and cheaper to modify without changing its observable behavior
- (verb): to restructure software by applying a series of
refactorings without changing its observable behavior.
Note: series of refactorings - ท่าที่จัดการ Code Smells ที่ละท่าา เช่น พวกแยก Extract Method ทำจนเราพอใจ แต่ตอน Apply Re-run Test หรือ ไม่โดน Tools อย่าง Sonar มันด่า 5555
Refactoring != Rewrite
- When to Refactor?
- Code เดิมต้องมี Test และ Test มันทำงานได้ด้วย //Run ผ่าน
- ทำระหว่าง Development ควรทำระหว่าง Dev ไปเลย มันจะเหมาะกับแนวคิด TDD
//ส่วนตัว ถ้ายังไม่ใช้ TDD ปรับยาก
//หวังว่า Non-IT จะมาอ่านนะ บอกไปทำที่หลัง ที่หลังพี่ไม่มารับผิดชอบ กันเวลา 555 ในเลข 5 นั้นมีน้ำตา
- Why Refactor?
- Economics - Business Value ทำแล้วลดเวลาปัญหาจุกจิ Scale ได้ / Technical Debt
//บ่นต่อและ หลายที่ Business บอกว่าไร้สาระ อย่างที่ผมทำ ปีนี้บาง Product น่าจะได้รับ Effect หลังจากดียมาได้ 5-6 ปี - ส่วนอื่นๆที่ได้ นอกจาก Clean Code / Quality เป็นต้น
แล้ว ถ้าไม่ทำเกิดอะไรขึ้น ตามนี้เลย Design Stamina Hypothesis (martinfowler.com) มีคน proof มาแล้ว
- General Step for Refactoring
- Find some code smell
- Make sure you have solid set of tests for that section of code and passing
สั้นๆ ทำ Test ส่วนตัวอาจจะยอมทำ Integration Test ของผมยอมทำแบบนั้นนะ ถ้างานมันรีบๆ ทำ1-2 เคส แล้วพอมี defect มาดู Coverage Report ถ้ามันครอบคลุม กล้า Refactor - Make a small code change by using Refactoring Techniques //NOT rewrite
- Make sure the tests pass
- Commit Code to add Check Point เผื่อพลาดจะได้ Revert ได้
- Repeat the TDD cycle until the smell is gone
ส่วนตัว ทำซ้ำ แต่อาจจะไม่ต้องใช้ TDD //ผมเขียนให้เห็นภาพ แล้วเอา Test ครอบที่หลัง
Refactoring Techniques
แต่ละ Code Smell จะมีเทคนิคที่ใช้จัดการต่างๆกัน แต่อันนี้จะใช้เน้น 4-5 ตัว
- Inline Function
แยก method จนตอนอ่านเรางงเอง ว่า Context มัน คือ อะไร เรียกว่ายังไงดี อาจจะใช้ท่า Extract Function มาไปจนมันเป็น Smell เอง
Note: แต่ Method ที่ทำ Inline ต้องไม่เป็น polymorphism นะ
- Before
func getPrice(person Person) string {
if isChildren(person) {
return "free"
}
return "$15"
}
func isChildren(person Person) string {
return person.age < 12
}- After
func getPrice(person Person) string {
if person.age < 12 {
return "free"
}
return "$15"
}เอาจริงๆ ผมชอบตัวอย่างของ Inline Method (refactoring.guru) มากกว่านะ มันอ่านแล้วเอ๊ะ สัมพันธ์กันยังไง
- Extract Function
เมื่อไหร่ที่เรา Comment ในส่วนนั้น หรือ ของ C# ใส่ Region ครอบ แตกมันออกมา แยกให้อ่านง่าย แล้วพอทำมันเล็กลง เราจะเขียน unit test ได้ง่ายขึ้น
- Before
func printReceipt(person Person) {
amount := getPrice(person)
//receipt details
fmt.Println("name:", person.Name)
fmt.Println("total amount:", amount)
}- After
func printReceipt(person Person) {
amount := getPrice(person)
receiptDetails(person, amount)
}
func receiptDetails(person, amount) {
fmt.Println("name:", person.Name)
fmt.Println("total amount:", amount)
}- Replace Nested Conditional with Guard Clauses
เงื่อนไขที่ซ้บซ้อนมันอ่าน Code ยาก จัดใหม่ มอง if ไปอีกมุม และทำ Guard Clauses ลด Nested If ลด Cognative Load ตอนอ่าน
- Before
func getPayAmount() int {
result := 0 //Code smell tmp variable
if isDead() {
result = deadAmount()
} else {
if isSeparated() {
result = separatedAmount()
} else {
if isRetired() {
result = retiredAmount()
} else {
result = normalPayAmount()
}
}
}
return result
}- After
func getPayAmount() int {
if isDead() {
return deadAmount()
}
if isSeparated() {
return separatedAmount()
}
if isRetired() {
return retiredAmount()
}
return normalPayAmount()
}- Elimitnate tmp variable (Inline Temp)
จากตัวอย่าง Replace Nested Conditional with Guard Clauses เจอตัวแปร result ไม่จำเป็นต้องใช้ เราสามารถ return ได้เลย จริงๆ พวกตัวแปร tmp มันจะมีปัญหาอีกอันนึงด้วย มันหาจุดแยก method ยาก มันใช้ตัวแปรเดียวกัน
- Early return
เหมือนกันมอง if ไปอีกมุม จริงๆ Software Eng จะมี Metric cyclomatic complexity ยิ่ง จุดตัดสินใจเยอะ ค่ายิ่งสูง ตัว Early return จะมาช่วยลดอันนี้ด้วย
- Before
func totalAmount(prices []int) int {
if len(prices) != 0 {
total := 0
for i := 0; i < len(prices); i++ {
total += prices[i]
}
return total
}
return 0
}- After
func totalAmount(prices []int) int {
if len(prices) == 0 {
return 0
}
total := 0
for i := 0; i < len(prices); i++ {
total += prices[i]
}
return total
}ที่เรียนมา ใช้หมดนะ Extract Function / Replace Nested Conditional with Guard Clauses / Early return แต่มีตัวนึงที่ผมใช้เองน้อยมากๆ หรือ แทบไม่ได้ใช้ Inline Function
TDD
จริงแล้ว มันเทคนิคนึงใน Extrame Programming
- TDD mantra (State)
- RED - Test ที่ Fail แต่มันบอกว่าเราต้องการ Test อะไร
- GREEN - Code ตอนแรกที่ไม่มี Logic ทำให้ Test ผ่าน เขียน Logic ตาม Requirement ก่อน และทำ Test
- Refactor ห้ามแก้ Code พร้อมกับ Test จะได้มีตัวกันแตก
- Code - เพิ่ม Logic ตาม Requirement ก่อน + หา Smell และปรับ
- Test - ปรับให้อ่านง่าย ลดความซับซ้อน เช่นจากเดิม Copy แปะ มาใช้ Testable
จากนั้นวน Loop ไป RED > GREEN > Refactor จนกว่าเราจะพอใจ หรือตี Constraint อื่นๆ เช่น เวลา
- Uncle Bob describes TDD with three rules:
- You are not allowed to write any production code unless it is to make a failing unit test pass.
- ทำให้ Fail (RED) ก่อน แล้วเขียน Logic (production code) รับประกันว่ามี Test แน่ๆ - You are not allowed to write any more of a unit test than is sufficient to fail, and compilation failures are failures.
- เขียน Test ให้พอดี และ Fail (RED) ทำที่ละอัน จะได้แก้ที่ละจุด (Small Change) - You are not allowed to write any more production code than is sufficient to pass the one failing unit test.
- ไม่ต้องคิดเยอะ คิดเผื่อ
Refactoring Sample
อันนี้ไม่มีอะไรมากครับ เป็นตัวอย่าง Refactor จาก Sample Project เช่น
- FizzBuzz - โจทย์ Classic ของ Test / TDD / Refactoring เลย
ผมเคยมีเขียนแบบ Java ด้วย ครบ 10 ปี พอดี 555 FizzBuzz Problem | naiwaen@DebuggingSoft สมัยนั้น่าจะ java 8 มาใหม่ๆ ตอนนี้ 21 และดีที่มาเรียนอันนี้ด้วย Blog ตอนนี้น่าจะแตกไปหลายปี 2 CSS พวก Syntax Highlight มันแตก ตอนนี้แก้เรียบร้อยแล้วอิอิ
- Billing Report - Refactor + Structure แยก Report Logic / Render
- Movie Rental - Go + Eliminate if with polymorphism
- Data Storage API - Dependency Injection for Test Support //อยากคิด Step ได้แบบ Speaker จริงๆ
v1. File Fixed Path Cannot Scale
v2. Introduce DI to eliminate relationship DB -> Storage (วิธีการสร้าง เนื้อ DB V1 มัน lock สร้าง file + fixed path)
v3. Parallel with os.createTemp
v4. In-Memory with mattetti/filebuffer //ตอนแรกงงๆ ต้องไปไล่ Commit ถ้าแยก Branch ก็ดี
v5. นอกจาก DB -> Storage ไปแล้ว ก็มาแก้ชั้น Handler -> Service -> DB แยก Interface แค่งงๆ ใช้ร่วมกันทั้ง Service / DB (น่าจะอันนี้ ถ้าพังไม่ผิด Go by Example: Struct Embedding)
|-----Interace Storage--------||-Interface io.ReadWriteSeeker-| Handler ---------> Service ---------> DB ---------> Storage
- Code Smell อื่นๆ และ
- Primitive Obsession แก้แล้ว พังเยอะ อาจจะทำใจด้วยว่า Test อาจจะไม่ช่วย หรือ ไม่ต้องถอดออกมาอีก Step Go Map
- Push up dependency - ทำให้ Simple อะไรที่ Class / Method นั้นไม่ควรรับรู้ เราผลักมันออกไป
- go polymorphism
ขยายจากตัว Movie Rental ลด if โดยเอา โดย
- แยกแต่ละ If ออกมาเป็น Class ใน Go Struct เพื่อสื่อ Movie กลุ่มต่างๆ
- ทำข้อตกลงที่แต่ละ Struct ทำงานเหมือนกัน โดยใช้ Interface พยายามหามันว่าอะไรที่ควรมี หา Common (Abstraction)
“Don't design with interfaces, discover them.”
Rob Pike
สุดท้าย Loop ที่มี if มันจะหายไป เพราะมันถูกซ่อนไปและ แต่อีกมุมมันแอบคล้าย Strategy Pattern นะ
- ปิดท้าย
Refactor นอกจาก Code Smell แล้ว อาจจะเป็นพวก Style Guide ของภาษาด้วย เช่น
- Struct ตัวแรกใหญ่ Public ตัวเล็ก Private ถ้าเจอ _ มันก็ private
- Return + Initialize Variable
- Get Set Func ไม่ต้องตั้งชื่อขึ้นต้นด้วย Get / Set ตามภาษาอื่นๆ ตั้งชื่อซ้ำกับ Struct ได้นะ
- Struct ตัวเล็ก
- Func ตัวใหญ่
ถ้า Struct ตัวใหญ่ + Func ตัวใหญ่ มันมีอะไรแปลกๆและ ทำ Get / Set Encapsulate ทำไม - print format message (eg. Sprintf) - ทำให้อ่านรู้เรื่อง ไม่จำเป็นต้อง Replace ทุกจุด
- ตัวแปร ถ้าไม่จำเป็นต้องเป็น Pointer มันไม่ต้องใช้นะ เพราะมันทำให้ Func มันไม่ Immutable ใครจะมาแก้ก็ได้ กระทบ Test
- iota ทำให้ Enum ของ Go มีลำดับ Id
- Run Test Parallel t.parallel ระวังปัญหา Race Condition Access Resource เดียวกัน เช่น ไฟล์ / DB แสดงว่าปกติ Test จะ Run ตามลำดับสินะ
Ref: Does go test run unit tests concurrently? - Stack Overflow - io.WriteSeeker / io.ReadSeeker เป็น interface ให้ IO ใช้ Data Storage API ตัวอย่างของอันนี้
แต่เราควรปรับตาม Style Guide ของภาษาที่ตกลงกันในทีม
ตอนไหนควร Refactor
- เงื่อนไขตัดสินใจ - เวลาที่มี / ความสำคัญ Code นั้นกับ Businesss / Impact
- เริ่มยังไง - ทำ Test ก่อน อาจจะกว้างๆ Integration / Unit Test (White Box/ Gray Box)
use before reuse - pattern เช่น duplication เช่น มี 3 จุด จะให้เรารู้ว่ามี Common / abstraction แล้ว เริ่มลุยได้
Boy Scout Rule - Leave your code better than you found it
หนังสือแนะนำจาก Coaching Session
- Refactoring: Improving the Design of Existing Code (2nd Edition) (Addison-Wesley Signature Series (Fowler)): Fowler, Martin: 9780134757599: Amazon.com: Books
- Effective Go - The Go Programming Language
และจะบอกว่าเรียนจบ แต่ยังเล่น Code ตามไม่จบ 5555 หยุดไว้ที่ FizzBuzz เดี๋ยวต้องลองไปฝึกอีกรอบก่อน
Reference
Discover more from naiwaen@DebuggingSoft
Subscribe to get the latest posts sent to your email.
