मेरी Best Practices – सामान्य

1. कमेंट्स – वरदान भी, अभिशाप भी

अच्छे कमेंट्स लिखना एक कला है। सोच-समझकर लिखा कमेंट काम आता है, वरना अक्सर बेकार ही जाता है।

Self-documenting कोड का मतलब “कोई कमेंट नहीं” नहीं है। कोड को जहाँ तक हो सके कैसे खुद बताना चाहिए; कमेंट्स उसमें क्यों, आसानी से न दिखने वाली शर्तें और किसी API का कॉन्ट्रैक्ट जोड़ते हैं।

आगे self-documenting कोड के कुछ उदाहरण हैं, साथ ही अच्छे और बुरे कमेंट्स भी।

एक आम दलील यह है कि कमेंट्स जल्दी outdated हो जाते हैं। असल में यह समस्या variables और methods के नामों पर भी उतनी ही लागू होती है!

उफ़ – ऐसा कभी न करें:

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

Self-documenting: variables के सार्थक नाम आधी जीत हैं

बुरा:

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: methods के सार्थक नाम

बुरा:

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

अच्छा:

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

Self-documenting: private methods

बुरा: यह method पढ़ने वाले को validation, calculation और business rule – तीनों को एक साथ समझने पर मजबूर करता है।

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 method पूरा flow बताता है। Private, stateless methods हर अलग इरादे को एक नाम देते हैं और अपनी dependencies को parameters और return values के ज़रिए साफ़ दिखाते हैं।

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));
}

हर लाइन को अपना अलग method नहीं चाहिए। Extract वही किया जाता है जो एक जुड़ा हुआ विचार हो और जिसे सटीक नाम दिया जा सके। वहीं 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 loading spinner को दिखाने और छिपाने वाले methods हैं। लेकिन methods चार हैं। क्यों, यह पहली नज़र में पूरी तरह साफ़ नहीं है:

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 = '';
}

सारे methods बहुत मिलते-जुलते लगते हैं और किसी न किसी तरह CSS के ज़रिए visibility बदलते हैं। एक छोटे-से कमेंट से उनके use cases तुरंत साफ़ हो जाते हैं:

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 से आम तौर पर बचना चाहिए (parameters का nightmare, हमेशा outdated और मेंटेन करने में झंझट) और इसका इस्तेमाल तभी करना चाहिए, जब किसी parameter को सचमुच ज़्यादा व्याख्या की ज़रूरत हो, या वरना पूरा लंबा method पढ़ना पड़े।

मूल नियम का अपवाद: क्या हो रहा है, उस पर कमेंट करें – यहाँ यही समझदारी है

जब इससे पढ़ना काफ़ी आसान हो जाए, क्योंकि वरना कोड साफ़ समझ नहीं आता:

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 से एक मज़ेदार किस्सा – source code में मौजूद सारी गालियों की गिनती:

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

कमेंट्स लिखने का मज़ा लीजिए!

2. बड़े routines में private methods ख़तरनाक हैं

सावधान! आज की object-oriented दुनिया में private methods हमेशा बहुत होते हैं। आम तौर पर यह कोई समस्या नहीं है। समस्या तब बनती है, जब ये methods fields, यानी state, को access करते हैं और बड़े routines का हिस्सा होते हैं। ऐसे sub-methods private और stateless होने चाहिए। वरना उनका नुकसान यह है कि अकेले call किए जाने पर वे अक्सर परेशानी खड़ी कर देते हैं। डेवलपर्स के मन में किसी न किसी दिन sub-method को सीधे call करने का ख़याल आएगा ही। Illegal states bugs की सबसे बड़ी वजहों में से एक हैं। इस बारे में John Carmack (id Software, Doom) के कुछ विचारों पर एक दिलचस्प लेख है, जिसमें यह भी बताया गया है कि जिन applications में कोड का बेहद स्थिर होना ज़रूरी है (जैसे अंतरिक्ष उड़ान), वहाँ अक्सर subroutines पर रोक होती है, जिससे कोड ज़्यादा deterministic बनता है: http://number-none.com/blow/john_carmack_on_inlined_code.html

यह किस्सा आज की stateless Java/Spring दुनिया पर कुछ हद तक ही लागू होता है, फिर भी सोचने के लिए अच्छी सामग्री है।