माझ्या 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 जगाला काही प्रमाणातच लागू होतो, पण तरीही विचार करायला लावणारा आहे.