Οι βέλτιστες πρακτικές μου – Γενικά

1. Σχόλια – κατάρα και ευλογία

Τα καλά σχόλια είναι τέχνη. Γραμμένα με σύνεση, είναι χάρισμα· συχνά όμως, κόπος χαμένος.

Ο αυτοτεκμηριωμένος κώδικας δεν σημαίνει «καθόλου σχόλια». Ο κώδικας πρέπει να εκφράζει όσο γίνεται μόνος του το πώς· τα σχόλια συμπληρώνουν το γιατί, τους μη προφανείς περιορισμούς και το συμβόλαιο ενός API.

Ακολουθούν μερικές περιπτώσεις αυτοτεκμηριωμένου κώδικα, καθώς και καλών και κακών σχολίων.

Ένα συχνό επιχείρημα είναι ότι τα σχόλια ξεπερνιούνται εύκολα. Στην πραγματικότητα, το ίδιο πρόβλημα ισχύει εξίσου για τα ονόματα μεταβλητών και μεθόδων!

Άουτς – μην το κάνεις ποτέ αυτό:

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

Αυτοτεκμηριωμένος κώδικας: τα εύστοχα ονόματα μεταβλητών είναι η μισή δουλειά

Λάθος:

var dateTime = DateTimeUtil.getNow();

Σωστά:

var now = DateTimeUtil.getNow();

Ένα ακόμη παράδειγμα:

Λάθος:

User user = this.getBruce();

Λάθος:

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

Σωστά:

User userWithoutPermission = this.getBruce();

Αυτοτεκμηριωμένος κώδικας: εύστοχα ονόματα μεθόδων

Λάθος:

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

Σωστά:

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

Αυτοτεκμηριωμένος κώδικας: ιδιωτικές μέθοδοι

Λάθος: η μέθοδος αναγκάζει τον αναγνώστη να αποκωδικοποιήσει ταυτόχρονα την επικύρωση, τον υπολογισμό και τον επιχειρησιακό κανόνα.

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

Σωστά: η δημόσια μέθοδος αφηγείται τη ροή. Οι ιδιωτικές, stateless μέθοδοι δίνουν όνομα σε κάθε επιμέρους πρόθεση και κάνουν ορατές τις εξαρτήσεις τους μέσω παραμέτρων και τιμών επιστροφής.

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

Ένα ακόμη παράδειγμα από την πράξη. Βλέπουμε μεθόδους που εμφανίζουν και κρύβουν τον δείκτη φόρτωσης (spinner) του 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) {}

Συζήτηση στο workshop της 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. Οι ιδιωτικές μέθοδοι σε μεγάλες ρουτίνες είναι επικίνδυνες

Προσοχή! Στον σημερινό αντικειμενοστραφή κόσμο υπάρχουν πάντα πολλές ιδιωτικές μέθοδοι. Αυτό συνήθως δεν είναι πρόβλημα. Γίνεται πρόβλημα μόνο όταν αυτές οι μέθοδοι προσπελαύνουν πεδία, δηλαδή την κατάσταση (state), και βρίσκονται μέσα σε μεγάλες ρουτίνες. Τέτοιες υπομέθοδοι πρέπει να είναι ιδιωτικές και stateless. Αλλιώς έχουν το μειονέκτημα ότι, αν κληθούν μεμονωμένα, συχνά δημιουργούν μπελάδες. Κάποιος προγραμματιστής θα σκεφτεί να καλέσει την υπομέθοδο. Οι μη έγκυρες καταστάσεις είναι μία από τις κύριες αιτίες των bugs. Ένα ενδιαφέρον άρθρο σχετικά, για κάποιες δηλώσεις του John Carmack (id Software, Doom) και για το ότι σε εφαρμογές όπου ο κώδικας πρέπει να είναι πολύ σταθερός (π.χ. διαστημικές πτήσεις) οι υπορουτίνες συχνά απαγορεύονται, κάτι που κάνει τον κώδικα πιο ντετερμινιστικό: http://number-none.com/blow/john_carmack_on_inlined_code.html

Η ιστορία αυτή εφαρμόζεται μόνο εν μέρει στον σύγχρονο, stateless κόσμο της Java/Spring, αλλά αποτελεί παρ’ όλα αυτά καλή τροφή για σκέψη.