Code Smell: Object-Orientation Abusers พร้อมตัวอย่าง Java

• ภาพรวม
• 1. Switch Statements: เงื่อนไขที่รวมพฤติกรรมหลายประเภทไว้ด้วยกัน
  • สัญญาณและแนวทางจากแหล่งอ้างอิง
  • ตัวอย่างก่อนปรับปรุง
  • ตัวอย่างหลังปรับปรุงด้วย Strategy
• 2. Temporary Field: ฟิลด์ที่มีความหมายเฉพาะบางช่วง
  • สัญญาณและแนวทางจากแหล่งอ้างอิง
  • ตัวอย่างก่อนปรับปรุง
  • ตัวอย่างหลังปรับปรุงด้วย Extract Class
  • ฟิลด์ที่ว่างไม่ได้เป็น smell ทุกกรณี
• 3. Refused Bequest: คลาสลูกปฏิเสธสิ่งที่สืบทอดมา
  • สัญญาณและแนวทางจากแหล่งอ้างอิง
  • ตัวอย่างก่อนปรับปรุง
  • ตัวอย่างหลังปรับปรุง: แยกความสามารถ
  • ความเชื่อมโยงกับ LSP และ ISP
• 4. Alternative Classes with Different Interfaces: หน้าที่เดียวกัน แต่เรียกใช้ต่างกัน
  • สัญญาณและแนวทางจากแหล่งอ้างอิง
  • ตัวอย่างก่อนปรับปรุง
  • ตัวอย่างหลังปรับปรุงด้วย interface ร่วมและ Adapter
• เลือกวิธีแก้ตามสาเหตุ
• แนวทางฝึกปฏิบัติ
• แหล่งอ้างอิง

การใช้คลาสและการสืบทอดไม่ได้รับประกันว่าระบบจะมีโครงสร้างเชิงวัตถุที่เหมาะสมเสมอไป กลุ่ม Object-Orientation Abusers ชวนให้ตรวจสอบว่าเราแบ่งพฤติกรรม เก็บสถานะ และกำหนดความสัมพันธ์ของคลาสสอดคล้องกับหน้าที่จริงหรือไม่

บทความนี้อธิบาย 4 Code Smell ตามหมวดของ Refactoring.Guru ได้แก่ Switch Statements, Temporary Field, Refused Bequest และ Alternative Classes with Different Interfaces พร้อมตัวอย่าง Java ที่เขียนขึ้นใหม่เพื่อใช้ในการเรียนการสอน [1–4]

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

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

#ภาพรวม

#1. Switch Statements: เงื่อนไขที่รวมพฤติกรรมหลายประเภทไว้ด้วยกัน

#สัญญาณและแนวทางจากแหล่งอ้างอิง

switch หรือชุด if/else ที่ซับซ้อน โดยเฉพาะเงื่อนไขแยกประเภทซึ่งกระจายหลายแห่ง อาจทำให้การเพิ่มประเภทใหม่ต้องตามแก้หลายจุด แนวทางหนึ่งคือแยกพฤติกรรมไปยังวัตถุและใช้ Polymorphism ส่วนเงื่อนไขสั้นที่ทำงานง่ายอาจคงไว้ได้ [1]

#ตัวอย่างก่อนปรับปรุง

class ShippingCalculator {
    int fee(String type, int weightGrams) {
        if (weightGrams <= 0) {
            throw new IllegalArgumentException("Weight must be positive");
        }
        return switch (type) {
            case "STANDARD" -> weightGrams <= 1000 ? 40 : 60;
            case "EXPRESS" -> weightGrams <= 1000 ? 70 : 100;
            default -> throw new IllegalArgumentException("Unknown type");
        };
    }
}

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

#ตัวอย่างหลังปรับปรุงด้วย Strategy

interface ShippingPolicy {
    int fee(int weightGrams);
}

class StandardShipping implements ShippingPolicy {
    @Override
    public int fee(int weightGrams) {
        return weightGrams <= 1000 ? 40 : 60;
    }
}

class ExpressShipping implements ShippingPolicy {
    @Override
    public int fee(int weightGrams) {
        return weightGrams <= 1000 ? 70 : 100;
    }
}

class ShippingCalculator {
    private final ShippingPolicy policy;

    ShippingCalculator(ShippingPolicy policy) {
        this.policy = java.util.Objects.requireNonNull(policy);
    }

    int fee(int weightGrams) {
        if (weightGrams <= 0) {
            throw new IllegalArgumentException("Weight must be positive");
        }
        return policy.fee(weightGrams);
    }
}

การใช้งานในเมธอดของโปรแกรม:

var calculator = new ShippingCalculator(new ExpressShipping());
System.out.println(calculator.fee(1500)); // 100

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

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

สิ่งที่ควรตรวจสอบ: ผลลัพธ์ของทุกประเภทที่น้ำหนัก 1, 1000 และ 1001 กรัม รวมถึงการปฏิเสธน้ำหนักศูนย์และค่าติดลบ

#2. Temporary Field: ฟิลด์ที่มีความหมายเฉพาะบางช่วง

#สัญญาณและแนวทางจากแหล่งอ้างอิง

ฟิลด์ที่จำเป็นเฉพาะสถานการณ์หนึ่ง แต่ไม่มีความหมายในช่วงอื่น อาจทำให้สถานะของวัตถุเข้าใจยาก หากฟิลด์หลายตัวทำงานร่วมกับอัลกอริทึมหนึ่ง สามารถใช้ Extract Class หรือ Method Object แยกข้อมูลและการทำงานออกไปได้ ส่วน Null Object เหมาะเฉพาะกรณีที่การไม่มีวัตถุมีพฤติกรรมทดแทนที่ชัดเจน [2]

#ตัวอย่างก่อนปรับปรุง

import java.util.List;

class ScoreReport {
    private List<Integer> workingScores;

    String summarize(List<Integer> scores) {
        if (scores.isEmpty()) {
            throw new IllegalArgumentException("Scores must not be empty");
        }
        workingScores = scores;
        double mean = average();
        int highest = maximum();
        workingScores = null;
        return "mean=" + mean + ", max=" + highest;
    }

    private double average() {
        return workingScores.stream()
                .mapToInt(Integer::intValue).average().orElseThrow();
    }

    private int maximum() {
        return workingScores.stream()
                .mapToInt(Integer::intValue).max().orElseThrow();
    }
}

workingScores ไม่ใช่ข้อมูลประจำตัวของรายงาน แต่เป็นข้อมูลของการเรียก summarize() ครั้งหนึ่ง เมธอดช่วยเหลือต้องอาศัยการกำหนดค่าฟิลด์ก่อนใช้งาน หากเกิด exception ก่อนล้างค่า ฟิลด์อาจค้างอยู่ และการเรียกบน instance เดียวกันพร้อมกันอาจรบกวนกันได้

#ตัวอย่างหลังปรับปรุงด้วย Extract Class

import java.util.List;

record ScoreAnalysis(List<Integer> scores) {
    ScoreAnalysis {
        scores = List.copyOf(scores);
        if (scores.isEmpty()) {
            throw new IllegalArgumentException("Scores must not be empty");
        }
    }

    double average() {
        return scores.stream()
                .mapToInt(Integer::intValue).average().orElseThrow();
    }

    int maximum() {
        return scores.stream()
                .mapToInt(Integer::intValue).max().orElseThrow();
    }
}

class ScoreReport {
    String summarize(List<Integer> scores) {
        var analysis = new ScoreAnalysis(scores);
        return "mean=" + analysis.average()
                + ", max=" + analysis.maximum();
    }
}

เมื่อใช้คะแนน [60, 80, 100] ทั้งสองแบบให้ข้อความ mean=80.0, max=100 แต่แบบหลังให้ข้อมูลของแต่ละงานอยู่ใน ScoreAnalysis ของงานนั้น และ ScoreReport ไม่ต้องมีฟิลด์ทำงานชั่วคราว List.copyOf() ยังป้องกันการแก้รายการผ่าน reference ต้นฉบับหลังสร้างวัตถุ

สำหรับงานที่สั้นกว่านี้ การใช้ตัวแปร local และส่งพารามิเตอร์เข้าเมธอดช่วยเหลืออาจเพียงพอ ไม่จำเป็นต้อง Extract Class เสมอไป ตัวอย่างสมมติให้รายการและสมาชิกไม่เป็น null; แบบหลังจะปฏิเสธสมาชิก null ตั้งแต่สร้าง ScoreAnalysis

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

#ฟิลด์ที่ว่างไม่ได้เป็น smell ทุกกรณี

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

#3. Refused Bequest: คลาสลูกปฏิเสธสิ่งที่สืบทอดมา

#สัญญาณและแนวทางจากแหล่งอ้างอิง

คลาสลูกที่รับเมธอดหรือข้อมูลมาแล้วไม่ต้องการใช้ โดยเฉพาะการ override เพื่อปฏิเสธการทำงาน อาจสะท้อนว่าลำดับชั้นไม่เหมาะสม หากความสัมพันธ์ไม่สมเหตุสมผลสามารถเปลี่ยนเป็น Delegation หรือปรับคลาสแม่ให้เหลือส่วนร่วมที่เหมาะสมได้ [3]

#ตัวอย่างก่อนปรับปรุง

class Document {
    private String content;

    Document(String content) {
        this.content = content;
    }

    public String read() {
        return content;
    }

    public void update(String content) {
        this.content = content;
    }
}

class ReadOnlyDocument extends Document {
    ReadOnlyDocument(String content) {
        super(content);
    }

    @Override
    public void update(String content) {
        throw new UnsupportedOperationException("Read only");
    }
}

สมมติว่า Document มีสัญญาว่าทุก instance แก้ไขเนื้อหาได้ เมธอดที่รับ Document จึงมีเหตุผลจะเรียก update() แต่เมื่อส่ง ReadOnlyDocument เข้าไป โปรแกรมกลับล้มเหลว ความสัมพันธ์นี้จึงไม่รองรับความคาดหวังของผู้เรียก

#ตัวอย่างหลังปรับปรุง: แยกความสามารถ

interface ReadableDocument {
    String read();
}

interface EditableDocument extends ReadableDocument {
    void update(String content);
}

record ReadOnlyDocument(String content) implements ReadableDocument {
    @Override
    public String read() {
        return content;
    }
}

class DraftDocument implements EditableDocument {
    private String content;

    DraftDocument(String content) {
        this.content = content;
    }

    @Override
    public String read() {
        return content;
    }

    @Override
    public void update(String content) {
        this.content = content;
    }
}

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

ตัวอย่างนี้ปรับ public contract จึงต้องย้ายผู้เรียกจาก Document ไปยัง interface ที่ตรงกับงานด้วย หากต้องคง API เดิมไว้ชั่วคราว ควรวางแผนย้ายผู้เรียกเป็นขั้นตอน หากต้องการเพียงใช้โค้ดบางส่วนร่วมกัน สามารถเก็บ collaborator ไว้เป็นฟิลด์แล้ว delegate แทนการสืบทอด

สิ่งที่ควรตรวจสอบ: เอกสารทั้งสองชนิดอ่านได้ และเฉพาะ DraftDocument ใช้ผ่าน EditableDocument เพื่อแก้ไขได้

#ความเชื่อมโยงกับ LSP และ ISP

ตัวอย่างนี้ใช้หลัก Liskov Substitution Principle (LSP) ตรวจสอบว่าชนิดลูกแทนชนิดแม่ตามสัญญาได้หรือไม่ และใช้ Interface Segregation Principle (ISP) ช่วยแยกสัญญาการอ่านกับการแก้ไข

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

#4. Alternative Classes with Different Interfaces: หน้าที่เดียวกัน แต่เรียกใช้ต่างกัน

#สัญญาณและแนวทางจากแหล่งอ้างอิง

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

#ตัวอย่างก่อนปรับปรุง

class SmsSender {
    String send(String phone, String message) {
        return phone + ":" + message;
    }
}

class LegacySmsClient {
    String transmit(String message, String phone) {
        return phone + ":" + message;
    }
}

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

#ตัวอย่างหลังปรับปรุงด้วย interface ร่วมและ Adapter

interface MessageSender {
    String send(String phone, String message);
}

class SmsSender implements MessageSender {
    @Override
    public String send(String phone, String message) {
        return phone + ":" + message;
    }
}

class LegacySmsClient {
    String transmit(String message, String phone) {
        return phone + ":" + message;
    }
}

class LegacySmsAdapter implements MessageSender {
    private final LegacySmsClient client;

    LegacySmsAdapter(LegacySmsClient client) {
        this.client = java.util.Objects.requireNonNull(client);
    }

    @Override
    public String send(String phone, String message) {
        return client.transmit(message, phone);
    }
}

ผู้เรียกใช้ MessageSender.send(phone, message) ได้เหมือนกันทั้งสองกรณี ส่วน Adapter รับผิดชอบแปลงการเรียกไปยัง API เดิม รวมถึงลำดับพารามิเตอร์

หากควบคุมซอร์สของทั้งสองคลาสได้ การ Rename Method และปรับ signature อาจตรงไปตรงมากว่า Adapter แต่หากเป็นไลบรารีภายนอกหรือ API ที่ผู้ใช้อื่นพึ่งพา Adapter จะช่วยจำกัดการเปลี่ยนแปลงไว้ในจุดเดียว

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

สิ่งที่ควรตรวจสอบ: เมื่อส่ง phone="0812345678" และ message="Hello" ทั้งสอง implementation ต้องคืน 0812345678:Hello เพื่อจับข้อผิดพลาดเรื่องลำดับพารามิเตอร์ และควรตรวจสอบ error contract ของบริการจริงก่อนใช้งานแทนกัน

#เลือกวิธีแก้ตามสาเหตุ

#แนวทางฝึกปฏิบัติ

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

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

#แหล่งอ้างอิง

คำอธิบายสัญญาณและแนวทางพื้นฐานอ้างอิงจากหน้าต่อไปนี้ ตัวอย่าง Java การวิเคราะห์สัญญา และแบบฝึกปฏิบัติเป็นเนื้อหาที่เรียบเรียงขึ้นเพื่อบทความนี้

1. Refactoring.Guru. Switch Statements
2. Refactoring.Guru. Temporary Field
3. Refactoring.Guru. Refused Bequest
4. Refactoring.Guru. Alternative Classes with Different Interfaces