HS Hannes Schneidermayer Java & Web Developer

Meine Best Practices für Java- & Webentwicklung

Meine Best Practices – Allgemein

1. Kommentare – Fluch und Segen

Gute Kommentare sind eine Kunst. Bedacht eine Gunst, doch auch oft umsunst.

„Good code is self-documenting.“

In 20+ years of writing code for a living, this is the one phrase I’ve heard the most. It’s cliché. And like many clichés, it has a kernel of truth to it. But this truth has been so abused that most people who utter the phrase have no idea what it really means. Is it true? Yes. Does it mean you should never comment your code? No.

Source & Must Read: https://medium.com/free-code-camp/code-comments-the-good-the-bad-and-the-ugly-be9cc65fbf83

Nachfolgend einige Fälle für self-documenting Code, sowie gute und schlechte Kommentare.

Ein häufiges Argument ist, dass Kommentare gerne outdated werden. Tatsächlich gilt diese Problematik für Variablen- und Methodennamen gleichermaßen!

Ouch – Never do this:

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

Self-documenting: Sinnvolle Namen für Variablen sind die halbe Miete

Bad:

var dateTime = DateTimeUtil.getNow();

Good:

var now = DateTimeUtil.getNow();

Ein weiteres Beispiel:

Bad:

User user = this.getBruce();

Bad:

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

Good:

User userWithoutPermission = this.getBruce();

Self-documenting: Sinnvolle Namen für Methoden

Bad:

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

Good:

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

Self-documenting: Private Methoden

Bad:

return TestDeploymentUtil.createBaseWar()
                         .addAsResource(new File("../../base/faces-controller/target/classes/com/"), "com/")
                         .addAsResource(new File("target/classes/com/"), "com/")
                         .addAsResource(new File("target/test-classes/com/"), "com/");
                         .addAsResource(new File("src/test/resources/com/"), "com/");

Good:

return TestDeploymentUtil.createBaseWar()
                         // faces controller
                         .addAsResource(new File("../../base/faces-controller/target/classes/com/"), "com/")

                         // module sources
                         .addAsResource(new File("target/classes/com/"), "com/")
                         .addAsResource(new File("target/test-classes/com/"), "com/");

                         // module resources
                         .addAsResource(new File("src/test/resources/com/"), "com/");

Good:

public ModuleWarBuilder addFacesControllerJar() {
    this.webArchive.addAsResource(new File("../../base/faces-controller/target/classes/com/"), "com/");
    return this;
}

public ModuleWarBuilder addModuleSources() {
    this.webArchive.addAsResource(new File("target/classes/com/"), "com/")
                   .addAsResource(new File("target/test-classes/com/"), "com/");
    return this;
}

public ModuleWarBuilder addModuleResources() {
    this.webArchive.addAsResource(new File("src/test/resources/com/"), "com/");
    return this;
}

Faustregel: Kommentiere warum etwas passiert, statt was passiert

Es ist klar was hier passiert:

public String cleanInput(input string){
    input = strings.ReplaceAll(input, "^", "-")
    input = strings.ReplaceAll(input, "?", "_")
    ...
}

Aber warum passiert es?

Besser:

public String cleanInput(input string){
    // clean input so that it can be used in a regex
    input = strings.ReplaceAll(input, "^", "-")
    input = strings.ReplaceAll(input, "?", "_")
    ...
}

Ein weiteres real-life Beispiel. Gezeigt werden Methoden um den Ajax-Ladespinner ein- und auszublenden. Allerdings gibt es vier Methoden. Warum ist nicht ganz klar auf den ersten Blick:

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

Die Methoden scheinen alle sehr ähnlich zu sein und irgendwie die Sichtbarkeit via CSS zu manipulieren. Mit einem kurzen Kommentar werden die Anwendungsfälle direkt klar:

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

Ein weiteres Beispiel:

Bad:

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

Good:

/**
 * 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.
 */
function setToolTipText(text) {}

Disukssion im Workshop vom 2021-06-21: Geteilte Meinung aller Entwickler, dass JavaDoc generell zu vermeiden ist (Parameter-Nightmare, ständig outdated und nicht angenehm wartbar) und nur Anwendung finden sollte, wenn ein Parameter wirklich nähere Erklärung braucht bzw. man sonst die ganze, lange Methode lesen müsste.

Ausnahme zur Faustregel: Kommentiere was passiert – hier ist es ratsam

Wenn es die Lesbarkeit deutlich vereinfacht, weil der Code sonst nicht klar ist:

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 */
)

Kommentare – Schimpfwörter im Code des Linux-Kernel

Abschließend noch ein lustiger Schwank aus dem Linux Kernel – die Anzahl aller Schimpfwörter im Source Code:

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

Have fun writing comments!

2. Private Methoden in grossen Programmroutinen sind gefährlich

Achtung! In der heutigen objektorientierten Welt gibt es immer viele private Methoden. Dies ist normalerweise kein Problem. Es wird erst zu einem, wenn diese Methoden auf Felder, sprich den State, zugreifen und in grossen Routinen sind. Solche Sub-Methoden sollten private und stateless sein. Sonst haben sie den Nachteil, dass alleine gecallt oft für Ärger sorgen können. Entwickler werden auf die Idee kommen, die Sub-Methode zu callen. Illegale States sind einer der Hauptgründe für Bugs. Interessanter Artikel hierzu über einige Aussagen von John Carmack (id Software, Doom) und dass in Anwedungen, wo der Code sehr stabil sein muss (Weltraumflug zB), oft Subroutinen verboten sind, was den Code deterministischer macht: http://number-none.com/blow/john_carmack_on_inlined_code.html

Dieser SC chwank ist nur bedingt auf die moderne stateless Java/Spring-Welt anzuwenden, aber dennoch ein guter Denkanstoss.