Code Smells: Dispensables — ลดส่วนเกินในโค้ดด้วยตัวอย่าง Java

• ภาพรวมทั้ง 6 แบบ
• 1. Comments — เมื่อคอมเมนต์กำลังชดเชยโค้ดที่อ่านยาก
  • ลักษณะปัญหา
  • ก่อนปรับปรุง
  • หลังปรับปรุง
  • คอมเมนต์แบบใดควรเก็บไว้
• 2. Duplicate Code — กฎเดียวกันถูกดูแลหลายแห่ง
  • ลักษณะปัญหา
  • ก่อนปรับปรุง
  • หลังปรับปรุง
  • อย่ารวมโค้ดเพียงเพราะหน้าตาคล้ายกัน
• 3. Lazy Class — คลาสที่เพิ่มภาระมากกว่าคุณค่า
  • ลักษณะปัญหา
  • ก่อนปรับปรุง
  • หลังปรับปรุง
  • คลาสเล็กไม่ได้แปลว่าเป็น Lazy Class เสมอไป
• 4. Data Class — ข้อมูลอยู่ที่หนึ่ง แต่พฤติกรรมกระจายอีกที่หนึ่ง
  • ลักษณะปัญหา
  • ก่อนปรับปรุง
  • หลังปรับปรุง
  • DTO และ Java record ไม่จำเป็นต้องมี business logic
• 5. Dead Code — โค้ดที่ไม่มีบทบาทแล้ว
  • ลักษณะปัญหา
  • ก่อนปรับปรุง
  • หลังปรับปรุง
  • ตรวจให้ครบก่อนลบ
• 6. Speculative Generality — ออกแบบเผื่อจนเกินความต้องการจริง
  • ลักษณะปัญหา
  • ก่อนปรับปรุง
  • หลังปรับปรุง
  • แตกต่างจาก Dead Code อย่างไร
• ขั้นตอน Refactoring ที่นำไปใช้ได้
• กรณีตรวจสอบสำหรับตัวอย่าง
• แบบฝึกหัดสำหรับผู้เรียน
• เอกสารอ้างอิง

โค้ดที่ทำงานได้อาจยังมีส่วนที่ทำให้ผู้อ่านเสียเวลา เช่น คอมเมนต์ที่ต้องอธิบายโค้ดทุกบรรทัด กฎธุรกิจที่คัดลอกหลายแห่ง หรือคลาสที่สร้างเผื่ออนาคตแต่ไม่เคยได้ใช้ กลุ่ม Dispensables ช่วยชี้ให้เห็นส่วนเหล่านี้ เพื่อพิจารณาลดความซับซ้อนในการอ่านและบำรุงรักษา

บทความนี้อธิบาย Code Smells ทั้ง 6 แบบตามหมวดของ Refactoring.Guru พร้อมตัวอย่าง Java ที่เขียนขึ้นใหม่ แนวทางแก้ไข และกรณีที่ควรเก็บโครงสร้างเดิมไว้

Code Smell เป็นสัญญาณให้ตรวจสอบบริบท ไม่ใช่ข้อพิสูจน์ว่าโค้ดผิด และไม่ใช่คำสั่งให้ลบโค้ดทันที

#ภาพรวมทั้ง 6 แบบ

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

#1. Comments — เมื่อคอมเมนต์กำลังชดเชยโค้ดที่อ่านยาก

#ลักษณะปัญหา

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

#ก่อนปรับปรุง

class ShippingService {
    int calculate(int x, boolean y) {
        // ลูกค้าสมาชิกที่ซื้อครบ 1,000 บาทได้รับบริการส่งฟรี
        if (y && x >= 1000) {
            return 0;
        }
        // ค่าจัดส่งปกติ 50 บาท
        return 50;
    }
}

ปัญหาหลักคือ x และ y ไม่บอกความหมาย และเงื่อนไขยังไม่มีชื่อที่สะท้อนกฎธุรกิจ

#หลังปรับปรุง

class ShippingService {
    private static final int FREE_SHIPPING_THRESHOLD_BAHT = 1000;
    private static final int STANDARD_FEE_BAHT = 50;

    int calculateFeeBaht(int subtotalBaht, boolean member) {
        return qualifiesForFreeShipping(subtotalBaht, member)
                ? 0 : STANDARD_FEE_BAHT;
    }

    private boolean qualifiesForFreeShipping(int subtotalBaht,
                                            boolean member) {
        return member && subtotalBaht >= FREE_SHIPPING_THRESHOLD_BAHT;
    }
}

ใช้การตั้งชื่อให้ชัดและ Extract Method เพื่อให้เงื่อนไขสื่อความหมาย ผลลัพธ์ยังเหมือนเดิม เช่น สมาชิกซื้อ 1,000 บาทส่งฟรี แต่ผู้ที่ไม่ใช่สมาชิกยังเสียค่าจัดส่ง 50 บาท

#คอมเมนต์แบบใดควรเก็บไว้

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

// ใช้เวลาจากผู้ให้บริการชำระเงิน เพื่อให้ตรวจสอบย้อนหลังกับรายการต้นทางได้

อย่าลบคำอธิบายที่จำเป็นเพียงเพื่อให้จำนวนคอมเมนต์ลดลง

อ่านเพิ่มเติม: Comments — Refactoring.Guru

#2. Duplicate Code — กฎเดียวกันถูกดูแลหลายแห่ง

#ลักษณะปัญหา

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

#ก่อนปรับปรุง

class PriceService {
    int checkoutTotal(int subtotalBaht, boolean member) {
        int shippingBaht = (member && subtotalBaht >= 1000) ? 0 : 50;
        return subtotalBaht + shippingBaht;
    }

    int quotationTotal(int subtotalBaht, boolean member) {
        int shippingBaht = (member && subtotalBaht >= 1000) ? 0 : 50;
        return subtotalBaht + shippingBaht;
    }
}

ถ้าเปลี่ยนเกณฑ์ส่งฟรีแล้วแก้เฉพาะตอนชำระเงิน ใบเสนอราคาจะคิดต่างจากยอดจริง

#หลังปรับปรุง

class PriceService {
    private final ShippingService shipping = new ShippingService();

    int checkoutTotal(int subtotalBaht, boolean member) {
        return totalWithShipping(subtotalBaht, member);
    }

    int quotationTotal(int subtotalBaht, boolean member) {
        return totalWithShipping(subtotalBaht, member);
    }

    private int totalWithShipping(int subtotalBaht, boolean member) {
        return subtotalBaht + shipping.calculateFeeBaht(subtotalBaht, member);
    }
}

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

#อย่ารวมโค้ดเพียงเพราะหน้าตาคล้ายกัน

ก่อนรวม ให้ตรวจว่าเป็น กฎเดียวกันและมีเหตุผลให้เปลี่ยนพร้อมกัน หากเป็นกฎคนละเรื่อง การสร้างเมธอดกลางที่เต็มไปด้วย flag อาจทำให้เข้าใจยากกว่าเดิม การแยกเป็นคอมโพเนนต์ร่วมก็ไม่จำเป็นต้องใช้ inheritance เสมอไป

อ่านเพิ่มเติม: Duplicate Code — Refactoring.Guru

#3. Lazy Class — คลาสที่เพิ่มภาระมากกว่าคุณค่า

#ลักษณะปัญหา

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

#ก่อนปรับปรุง

class OrderLabel {
    String format(long orderId) {
        return "ORDER-" + orderId;
    }
}

class ReceiptPrinter {
    private final OrderLabel label = new OrderLabel();

    String header(long orderId) {
        return label.format(orderId);
    }
}

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

#หลังปรับปรุง

class ReceiptPrinter {
    String header(long orderId) {
        return "ORDER-" + orderId;
    }
}

ใช้ Inline Class ย้ายงานที่ยังจำเป็นกลับมาที่ผู้ใช้ แล้วลบคลาสที่ไม่เพิ่มคุณค่า หากปัญหาเป็น subclass ที่ไม่มีหน้าที่แตกต่าง อาจใช้ Collapse Hierarchy รวมระดับการสืบทอด

#คลาสเล็กไม่ได้แปลว่าเป็น Lazy Class เสมอไป

คลาสเล็กที่รักษา invariant แทนแนวคิดสำคัญ เช่น EmailAddress หรือเป็น adapter เชื่อมระบบภายนอก อาจมีหน้าที่ชัดเจน ควรประเมินประโยชน์ของขอบเขตดังกล่าว ไม่ใช้จำนวนบรรทัดเป็นเกณฑ์เดียว

อ่านเพิ่มเติม: Lazy Class — Refactoring.Guru

#4. Data Class — ข้อมูลอยู่ที่หนึ่ง แต่พฤติกรรมกระจายอีกที่หนึ่ง

#ลักษณะปัญหา

คลาสมีข้อมูลและ getters/setters แต่การตรวจสอบหรือเปลี่ยนข้อมูลตามกฎธุรกิจเกิดในผู้เรียกหลายแห่ง ทำให้เจ้าของข้อมูลควบคุมสถานะของตนเองไม่ได้ดี และกฎเดียวกันอาจถูกเขียนซ้ำ

#ก่อนปรับปรุง

class StockItem {
    private int quantity;

    StockItem(int quantity) {
        if (quantity < 0) {
            throw new IllegalArgumentException("Negative stock");
        }
        this.quantity = quantity;
    }

    int getQuantity() {
        return quantity;
    }

    void setQuantity(int quantity) {
        this.quantity = quantity;
    }
}

class InventoryService {
    void reserve(StockItem item, int amount) {
        if (amount <= 0) {
            throw new IllegalArgumentException("Amount must be positive");
        }
        if (item.getQuantity() < amount) {
            throw new IllegalStateException("Insufficient stock");
        }
        item.setQuantity(item.getQuantity() - amount);
    }
}

ผู้เรียกอื่นสามารถข้าม InventoryService แล้วเรียก setter เพื่อตั้งจำนวนติดลบได้ กฎการจองจึงไม่ได้ถูกบังคับผ่านเจ้าของข้อมูล

#หลังปรับปรุง

class StockItem {
    private int quantity;

    StockItem(int quantity) {
        if (quantity < 0) {
            throw new IllegalArgumentException("Negative stock");
        }
        this.quantity = quantity;
    }

    int getQuantity() {
        return quantity;
    }

    void reserve(int amount) {
        if (amount <= 0) {
            throw new IllegalArgumentException("Amount must be positive");
        }
        if (quantity < amount) {
            throw new IllegalStateException("Insufficient stock");
        }
        quantity -= amount;
    }
}

class InventoryService {
    void reserve(StockItem item, int amount) {
        item.reserve(amount);
    }
}

ใช้ Move Method ย้ายกฎเข้าหาข้อมูล และลบ setter ที่เปิดกว้างเกินไป สำหรับการจองที่ถูกต้องและกรณีปฏิเสธการจอง พฤติกรรมของ InventoryService.reserve ยังคงเดิม ส่วน API ของ StockItem ถูกจำกัดโดยเจตนา จึงต้องปรับผู้เรียก setter ทุกแห่ง

ตัวอย่างนี้อธิบายการจัดวางกฎในออบเจ็กต์เดียว ยังไม่ครอบคลุมการจองพร้อมกันหลายคำขอ ระบบจริงต้องจัดการ transaction และ concurrency ในชั้นที่เหมาะสมด้วย

#DTO และ Java record ไม่จำเป็นต้องมี business logic

คลาสสำหรับรับ request ส่ง response หรือแลกเปลี่ยนข้อมูลข้ามขอบเขตระบบ อาจตั้งใจให้เป็นข้อมูลล้วน เช่น

record ProductResponse(long id, String name, int quantity) {}

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

อ่านเพิ่มเติม: Data Class — Refactoring.Guru

#5. Dead Code — โค้ดที่ไม่มีบทบาทแล้ว

#ลักษณะปัญหา

ตัวแปร พารามิเตอร์ ฟิลด์ เมธอด หรือคลาสเลิกถูกใช้ อาจเหลือจากการเปลี่ยน requirement หรือแทนที่วิธีทำงานเดิม รวมถึงบางเส้นทางที่ไม่สามารถถูกเรียกได้แล้ว

#ก่อนปรับปรุง

class InvoiceService {
    String createInvoice(String customerName, boolean legacyMode) {
        int oldRetryLimit = 3;
        return "Invoice: " + customerName;
    }

    private String createLegacyInvoice(String customerName) {
        return "Legacy invoice: " + customerName;
    }
}

legacyMode ไม่ถูกอ่าน ตัวแปร oldRetryLimit ไม่ถูกใช้ และไม่มีผู้เรียกเมธอด private ที่เหลืออยู่

#หลังปรับปรุง

class InvoiceService {
    String createInvoice(String customerName) {
        return "Invoice: " + customerName;
    }
}

ลบตัวแปรและเมธอดที่ไม่ใช้ พร้อม Remove Parameter และปรับ call sites เช่น createInvoice("Naruapon", false) เป็น createInvoice("Naruapon")

#ตรวจให้ครบก่อนลบ

ผล Find Usages และคำเตือนจาก IDE เป็นหลักฐานเริ่มต้น แต่การไม่มี reference แบบตรง ๆ ยังไม่เพียงพอสำหรับโค้ดที่เรียกผ่าน reflection, dependency injection, annotations, configuration หรือผู้ใช้ภายนอกของ public API

นอกจากนี้ ตัวแปรที่ไม่มีใครอ่านอาจมี initializer ที่มีผลข้างเคียง เช่น int result = sendPayment(); การลบทั้งบรรทัดจะทำให้ sendPayment() ไม่ทำงาน ต้องแยกให้ชัดระหว่างผลลัพธ์ที่ไม่ถูกใช้กับการเรียกที่ยังจำเป็น

เมื่อยืนยันแล้วว่าไม่จำเป็น ให้ลบแทนการคอมเมนต์โค้ดทิ้งไว้ โดยใช้ประวัติใน version control เมื่อต้องตรวจย้อนหลัง

อ่านเพิ่มเติม: Dead Code — Refactoring.Guru

#6. Speculative Generality — ออกแบบเผื่อจนเกินความต้องการจริง

#ลักษณะปัญหา

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

#ก่อนปรับปรุง

abstract class AbstractReportExporter {
    abstract String export(String title);

    boolean supportsEncryption() {
        return false;
    }
}

class PlainTextReportExporter extends AbstractReportExporter {
    @Override
    String export(String title) {
        return "Report: " + title;
    }
}

สมมติว่ามีผู้ใช้ exporter เพียงแบบเดียว ไม่มีสัญญา plugin และไม่มีผู้เรียก supportsEncryption() ซึ่งเพิ่มไว้เผื่อฟีเจอร์ที่ยังไม่ได้วางแผน

#หลังปรับปรุง

class PlainTextReportExporter {
    String export(String title) {
        return "Report: " + title;
    }
}

ใช้ Collapse Hierarchy และลบ hook ที่ไม่มีผู้ใช้ พร้อมปรับผู้เรียกที่อ้างชนิด abstract เดิม เมื่อมีความต้องการหลายรูปแบบจริงจึงพิจารณา abstraction ที่เหมาะกับความแตกต่างนั้น

#แตกต่างจาก Dead Code อย่างไร

สองแบบนี้ซ้อนทับกันได้ เช่น hook ที่สร้างเผื่อและไม่มีผู้เรียกอาจเข้าข่ายทั้งคู่ แต่ การมี interface ที่มี implementation เดียวไม่ใช่ข้อพิสูจน์ว่าออกแบบเกินจำเป็น หากเป็นขอบเขตสำหรับทดแทน dependency การทดสอบ หรือสัญญาของ framework ที่มีผู้ใช้จริง ก็อาจเหมาะสม

อ่านเพิ่มเติม: Speculative Generality — Refactoring.Guru

#ขั้นตอน Refactoring ที่นำไปใช้ได้

1. ระบุปัญหาเป็นเหตุการณ์จริง เช่น เปลี่ยนเกณฑ์ส่งฟรีแล้วต้องแก้สามไฟล์ หรือผู้เรียก setter ทำให้สต็อกติดลบ
2. ตรวจขอบเขตการใช้งาน รวม call sites, configuration, tests และสัญญากับผู้ใช้ภายนอก
3. บันทึกพฤติกรรมเดิมที่ต้องรักษา ใช้ tests ที่มีอยู่ และเพิ่มกรณีสำคัญหากยังไม่มีหลักฐานเพียงพอ
4. แก้ทีละประเด็น เช่น แยกเมธอดก่อนรวมกฎซ้ำ หลีกเลี่ยงเปลี่ยนกฎธุรกิจพร้อมย้ายโค้ด
5. อัปเดตผู้เรียกและตรวจผล build รันชุดทดสอบที่เกี่ยวข้อง และทบทวน diff
6. ประเมินความเข้าใจ ผู้อ่านตามงานได้ง่ายขึ้นหรือไม่ และมี dependency ใหม่ที่ไม่จำเป็นเกิดขึ้นหรือไม่

สำหรับ public API อาจต้องมีช่วง deprecation หรือแผนเปลี่ยนเวอร์ชัน การลบหรือเปลี่ยน signature โดยตรงเหมาะเมื่อควบคุมผู้เรียกที่ได้รับผลกระทบได้ครบ

#กรณีตรวจสอบสำหรับตัวอย่าง

ตารางนี้เป็นเกณฑ์ตรวจสอบสำหรับทำแบบฝึกหัด ไม่ใช่รายงานผลการรัน Java ในสภาพแวดล้อมจริง

#แบบฝึกหัดสำหรับผู้เรียน

เลือกโค้ดในโครงการหนึ่งส่วน แล้วตอบคำถามต่อไปนี้

1. พบสัญญาณของ Dispensables แบบใด พร้อมแสดงจุดที่เป็นหลักฐาน
2. ถ้าไม่แก้ จะเกิดต้นทุนหรือข้อผิดพลาดอย่างไรเมื่อ requirement เปลี่ยน
3. เทคนิคใดแก้ปัญหาได้โดยเปลี่ยนน้อยที่สุด
4. มีเหตุผลใดที่ควรเก็บโครงสร้างเดิมไว้
5. จะพิสูจน์อย่างไรว่าพฤติกรรมที่ผู้ใช้ต้องการยังคงเดิม

สิ่งที่ส่ง: โค้ดก่อน–หลัง เหตุผลในการตัดสินใจ และหลักฐานการตรวจพฤติกรรมที่เกี่ยวข้อง เป้าหมายคือทำให้โค้ดดูแลได้ง่ายขึ้นโดยรักษาความถูกต้องของระบบ

#เอกสารอ้างอิง

บทความเรียบเรียงคำอธิบายภาษาไทยและสร้างตัวอย่าง Java ใหม่จากแนวคิดในแหล่งต่อไปนี้ โดยข้อพิจารณาเรื่อง DTO ขอบเขตระบบ concurrency และการตรวจผลเป็นคำอธิบายประกอบของบทความ

• Refactoring.Guru: Comments
• Refactoring.Guru: Duplicate Code
• Refactoring.Guru: Lazy Class
• Refactoring.Guru: Data Class
• Refactoring.Guru: Dead Code
• Refactoring.Guru: Speculative Generality