Best Practices ของผม – ทั่วไป

1. คอมเมนต์ – ทั้งคุณและโทษ

การเขียนคอมเมนต์ที่ดีคือศิลปะ ใส่อย่างใคร่ครวญก็มีค่า แต่หลายครั้งก็ไร้ราคา

Self-documenting Code ไม่ได้แปลว่า “ไม่มีคอมเมนต์” โค้ดควรบอกวิธีการได้ด้วยตัวเองให้มากที่สุด ส่วนคอมเมนต์มีไว้เสริมเหตุผล เงื่อนไขแวดล้อมที่มองไม่เห็นจากโค้ด และสัญญา (contract) ของ API

ต่อไปนี้คือตัวอย่างของ self-documenting code รวมถึงคอมเมนต์ที่ดีและไม่ดี

ข้อโต้แย้งที่ได้ยินบ่อยคือคอมเมนต์มักจะล้าสมัย แต่ที่จริงปัญหานี้ก็เกิดกับชื่อตัวแปรและชื่อเมธอดเหมือนกัน!

โอ๊ย – อย่าทำแบบนี้เด็ดขาด:

int age = 32; // set the value of the age integer to 32

Self-documenting: ตั้งชื่อตัวแปรให้มีความหมาย ก็สำเร็จไปครึ่งหนึ่งแล้ว

ไม่ดี:

var dateTime = DateTimeUtil.getNow();

ดี:

var now = DateTimeUtil.getNow();

อีกตัวอย่างหนึ่ง:

ไม่ดี:

User user = this.getBruce();

ไม่ดี:

// testing user without permission
User user = this.getBruce();

ดี:

User userWithoutPermission = this.getBruce();

Self-documenting: ตั้งชื่อเมธอดให้มีความหมาย

ไม่ดี:

public String sanitizeInput(String input) {
    // replace underscore with spaces
    return input.replaceAll("_", " ");
}

ดี:

public String replaceUnderscoreWithSpaces(String input) {
    return input.replaceAll("_", " ");
}

Self-documenting: เมธอดแบบ private

ไม่ดี: เมธอดนี้บังคับให้ผู้อ่านต้องแกะการตรวจสอบข้อมูล การคำนวณ และกฎทางธุรกิจไปพร้อม ๆ กัน

public Invoice createInvoice(Order order, Customer customer) {
    if (order.items().isEmpty()) {
        throw new IllegalArgumentException("Order needs at least one item");
    }

    BigDecimal subtotal = order.items().stream()
            .map(item -> item.unitPrice().multiply(BigDecimal.valueOf(item.quantity())))
            .reduce(BigDecimal.ZERO, BigDecimal::add);
    BigDecimal discount = customer.isPremium()
            ? subtotal.multiply(new BigDecimal("0.10"))
            : BigDecimal.ZERO;

    return new Invoice(order.id(), subtotal, discount, subtotal.subtract(discount));
}

ดี: เมธอด public เล่าลำดับการทำงาน ส่วนเมธอด private ที่เป็น stateless ตั้งชื่อตามเจตนาแต่ละข้อ และทำให้เห็น dependency ของมันผ่านพารามิเตอร์และค่าที่ส่งกลับ

public Invoice createInvoice(Order order, Customer customer) {
    requireItems(order);
    BigDecimal subtotal = calculateSubtotal(order.items());
    BigDecimal discount = loyaltyDiscountFor(customer, subtotal);

    return invoiceFor(order, subtotal, discount);
}

private static void requireItems(Order order) {
    if (order.items().isEmpty()) {
        throw new IllegalArgumentException("Order needs at least one item");
    }
}

private static BigDecimal calculateSubtotal(List<OrderItem> items) {
    return items.stream()
            .map(item -> item.unitPrice().multiply(BigDecimal.valueOf(item.quantity())))
            .reduce(BigDecimal.ZERO, BigDecimal::add);
}

private static BigDecimal loyaltyDiscountFor(Customer customer, BigDecimal subtotal) {
    return customer.isPremium()
            ? subtotal.multiply(new BigDecimal("0.10"))
            : BigDecimal.ZERO;
}

private static Invoice invoiceFor(
        Order order,
        BigDecimal subtotal,
        BigDecimal discount
) {
    return new Invoice(order.id(), subtotal, discount, subtotal.subtract(discount));
}

ไม่จำเป็นต้องแยกทุกบรรทัดออกเป็นเมธอดของตัวเอง สิ่งที่ควรแยกออกมาคือความคิดหนึ่งก้อนที่ต่อเนื่องกันและตั้งชื่อได้อย่างแม่นยำ ส่วนชื่ออย่าง processData() หรือ handleStuff() ไม่ได้ช่วยอะไรเลย

หลักง่าย ๆ: คอมเมนต์ว่าทำไมสิ่งนั้นจึงเกิดขึ้น แทนที่จะบอกว่าอะไรเกิดขึ้น

ตรงนี้เห็นได้ชัดว่าอะไรเกิดขึ้น:

public Duration retryDelay(int retryCount) {
    long seconds = Math.min(1L << retryCount, 60);
    return Duration.ofSeconds(seconds);
}

แต่ทำไมถึงเป็นแบบนั้น?

ดีกว่า:

public Duration retryDelay(int retryCount) {
    // Cap the delay so an outage never blocks queued jobs for more than one minute.
    long seconds = Math.min(1L << retryCount, 60);
    return Duration.ofSeconds(seconds);
}

อีกตัวอย่างจากงานจริง นี่คือเมธอดสำหรับแสดงและซ่อนไอคอนหมุนตอนโหลด Ajax แต่มีอยู่ถึงสี่เมธอด และเมื่อมองแวบแรกก็ไม่ชัดว่าทำไม:

function showAjaxLoader() {
    $('.layout-ajax-loader').children().first().show();
}

function hideAjaxLoader() {
    $('.layout-ajax-loader').children().first().hide();
}

function suppressAjaxLoader() {
    getOnlyElementByClassOrNull('layout-ajax-loader').style.display = 'none';
}

function releaseAjaxLoader() {
    getOnlyElementByClassOrNull('layout-ajax-loader').style.display = '';
}

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

showAjaxLoader: function() {
    $('.layout-ajax-loader').children().first().show();
},

hideAjaxLoader: function() {
    $('.layout-ajax-loader').children().first().hide();
},

// take the control from primefaces and don't allow the spinner to show, no matter what
suppressAjaxLoader: function() {
    getOnlyElementByClassOrNull('layout-ajax-loader').style.display = 'none';
},

// reset the state after hiding, to give the control back to primefaces
releaseAjaxLoader: function() {
    getOnlyElementByClassOrNull('layout-ajax-loader').style.display = '';
},

อีกตัวอย่างหนึ่ง:

ไม่ดี:

/**
 * Sets the tool tip text.
 *
 * @param text  the text of the tool tip
 */
public void setToolTipText(String text) {}

ดี:

/**
 * Registers the text to display in a tool tip. The text
 * displays when the cursor lingers over the component.
 *
 * @param text  the string to display. If the text is null,
 *              the tool tip is turned off for this component.
 */
public void setToolTipText(String text) {}

ข้อสรุปจากเวิร์กช็อปวันที่ 2021-06-21: นักพัฒนาทุกคนเห็นตรงกันว่าโดยทั่วไปควรหลีกเลี่ยง Javadoc (ฝันร้ายเรื่องพารามิเตอร์ ล้าสมัยอยู่ตลอด และดูแลรักษายาก) และควรใช้เฉพาะเมื่อพารามิเตอร์ต้องการคำอธิบายเพิ่มเติมจริง ๆ หรือเมื่อถ้าไม่มีแล้วจะต้องอ่านเมธอดยาว ๆ ทั้งหมด

ข้อยกเว้นของหลักข้างต้น: คอมเมนต์ว่าอะไรเกิดขึ้น – กรณีนี้ควรทำ

เมื่อช่วยให้อ่านง่ายขึ้นอย่างมาก เพราะถ้าไม่มีคอมเมนต์ โค้ดจะไม่ชัดเจน:

let isAlwaysAllowedKey = (
    e.which === 8  || /* BACKSPACE */
    e.which === 35 || /* END */
    e.which === 36 || /* HOME */
    e.which === 37 || /* LEFT */
    e.which === 38 || /* UP */
    e.which === 39 || /* RIGHT*/
    e.which === 40 || /* DOWN */
    e.which === 46 || /* DEL*/
    e.ctrlKey === true && e.which === 65 || /* CTRL + A */
    e.ctrlKey === true && e.which === 88 || /* CTRL + X */
    e.ctrlKey === true && e.which === 67 || /* CTRL + C */
    e.ctrlKey === true && e.which === 90   /* CTRL + Z */
)

คอมเมนต์ – คำหยาบในโค้ดของ Linux Kernel

ปิดท้ายด้วยเกร็ดขำ ๆ จาก Linux Kernel – จำนวนคำหยาบทั้งหมดในซอร์สโค้ด:

https://www.vidarholen.net/contents/wordcount/#fuck*,shit*,damn*,idiot*,retard*

ขอให้สนุกกับการเขียนคอมเมนต์!

2. เมธอด private ในรูทีนขนาดใหญ่เป็นอันตราย

ระวัง! ในโลกเชิงวัตถุทุกวันนี้มีเมธอด private อยู่มากมายเสมอ ซึ่งโดยปกติไม่ใช่ปัญหา ปัญหาจะเกิดก็ต่อเมื่อเมธอดเหล่านี้เข้าถึงฟิลด์ หรือก็คือ state และอยู่ในรูทีนขนาดใหญ่ เมธอดย่อยแบบนี้ควรเป็น private และ stateless ไม่เช่นนั้นเมื่อถูกเรียกแยกเดี่ยว ๆ ก็มักก่อปัญหา เพราะสักวันจะมีนักพัฒนาคิดเรียกเมธอดย่อยนั้นโดยตรง state ที่ไม่ถูกต้อง (illegal state) คือหนึ่งในสาเหตุหลักของบั๊ก มีบทความที่น่าสนใจเรื่องนี้ ซึ่งรวบรวมความเห็นบางส่วนของ John Carmack (id Software, Doom) และเล่าว่าในแอปพลิเคชันที่โค้ดต้องเสถียรสูงมาก (เช่น การบินอวกาศ) มักห้ามใช้ซับรูทีน ซึ่งทำให้โค้ดคาดเดาผลได้แน่นอน (deterministic) มากขึ้น: http://number-none.com/blow/john_carmack_on_inlined_code.html

เรื่องนี้นำมาใช้กับโลก Java/Spring สมัยใหม่ที่เป็น stateless ได้เพียงบางส่วน แต่ก็ยังเป็นข้อคิดที่ดี