Besseren Code schreiben

Dan Čermák

8. September 2022

2022

CC BY 4.0

who -u

Dan Čermák

Software Entwickler @SUSE
i3 SIG, Package Maintainer
Developer Tools, QA, Dokumentation, Heimautomatisierung
https://dancermak.name
dcermak / D4N
@DefolosDC
@Defolos@mastodon.social

Agenda

Disclaimer

There is no silver bullet.

Do as I say and not as I do.

Wann ist Code "schlecht"?

  • schlecht dokumentiert
  • inkonsistent
  • "pasta code"
  • ungetestet und/oder untestbar
  • Regressions

Komplexität hat überhand genommen

Komplexität

Unvermeidbar

nach Brooks:

  1. essential complexity
  2. accidental complexity

Komplexität proaktiv bekämpfen

  1. Modularisierung
  2. Abstraktion

Modularisierung

  • logisch abgeschlossene Einheit
  • exportiert ein (schmales) Interface

Abstraktion

  • Vereinfachung die unwichtiges weglässt
  • verstecken: Implementierungsdetails
  • verallgemeinern
  • Gefahren: leaky abstraction, false abstraction

Module entwerfen

  • Problem in Unabhängiges teilen
  • funktionell ähnliches identifizieren
  • komplexe Logik finden und "verstecken"

Getrennt oder Zusammen?

  • Duplizierung reduzieren
  • Interface vereinfachen
  • Informationsteilung
  • Allgemeiner- & Sonderfall

API entwerfen

  • simpel & einfach benutzbar entwerfen
  • gestalten für universelleren Einsatz
ssize_t write(int fd, const void *buf, size_t count);

Anti-Patterns, Code Smells und Strategien

Anti-patterns & Code smells:

Indikator für schlechtes Design

No Abstraction

Windows API: Service starten

BOOL StartServiceA(
  SC_HANDLE hService, /* handle from OpenService */
  DWORD     dwNumServiceArgs,     /* arguments */
  LPCSTR    *lpServiceArgVectors
);
BOOL QueryServiceStatusEx(
  SC_HANDLE      hService,
  SC_STATUS_TYPE InfoLevel, /* SC_STATUS_PROCESS_INFO */
  LPBYTE         lpBuffer,  /* & SERVICE_STATUS_PROCESS struct */
  DWORD          cbBufSize, /* sizeof(SERVICE_STATUS_PROCESS) */
  LPDWORD        pcbBytesNeeded
);
typedef struct _SERVICE_STATUS_PROCESS {
  DWORD dwServiceType;
  /* The current state of the service. */
  /* e.g.: SERVICE_RUNNING, SERVICE_START_PENDING */
  DWORD dwCurrentState;
  DWORD dwControlsAccepted;
  DWORD dwWin32ExitCode;
  DWORD dwServiceSpecificExitCode;
  /* The check-point value that the service increments */
  /* periodically to report its progress during a lengthy */
  /* start, stop, pause, or continue operation. */
  DWORD dwCheckPoint;
  /* The estimated time required for a pending start, */
  /* stop, pause, or continue operation, in milliseconds. */
  DWORD dwWaitHint;
  DWORD dwProcessId;
  DWORD dwServiceFlags;
} SERVICE_STATUS_PROCESS, *LPSERVICE_STATUS_PROCESS;

Implementation detail leakage

std::unordered_map<std::string, size_t>
get_line_occurences(const std::string_view& lines);

vs

struct line_occurence {
    size_t
    get_occurence(const std::string& line);
};
line_occurence
get_line_occurences(const std::string_view& lines);

Codeduplizierung

Duplizierungen vermeiden!

"beinahe" Codeduplizierung

Konsistenz

  • Namen ähnlich wählen
  • Style Guides folgen
  • Mehrdeutigkeiten immer gleich behandeln:
int read_data_in(const char* filename, struct data* payload);
int write_out_in(struct data* payload, int fast_mode);

Namen als Anhaltspunkte (1)

schlecht:

// post an event into the queue
bool post_event(const event_t& e);

// send an event into the queue
bool send_event(const event_t& e);

besser

// register an event from user mode
bool register_event_user_mode(const event_t& e);

// send an event into the queue
bool register_event_interrupt_mode(const event_t& e);

Namen als Anhaltspunkte (2)

def fetch_data(con: Connection) -> Optional[Data]:

vs

def get_data(con: Connection) -> Optional[Data]

API selber nutzen

  • Schmerzpunkte nur in Praxis gefunden

→ downstream Kontakt halten

oder selber downstream werden

Write obvious code

Everyone knows that debugging is twice as hard as writing a program in the first place. So if you're as clever as you can be when you write it, how will you ever debug it?

– Brian W. Kernighan

  • Code wird viel öfter gelesen als geschrieben

Typensystem

  • starke Typisierung macht code robuster & klarer
def read_data(fd):
    ...

def read_data(fd: Union[string, int]) -> DataRow:
    ...

Funktionale Programmierung

  • unveränderliche Datenstrukturen
  • pure functions
(* output = pure_func_call(input) *)
let eigenvalues = eigen_decomp matrix;;

OCaml

# let square x = x *. x;;
val square : float -> float = <fun>
# let res = square 2.5;;
val res : float = 6.25
# print_endline;;
- : string -> unit = <fun>
# let res = print_endline "foo";;
foo
val res : unit = ()

Dokumentation

nicht dokumentierbarer & dokumentierter Code = schlechter Code

  1. technische Dokumentation
  2. Tutorials
  3. How to guides
  4. Erläuterungen
  5. Architektur

API Dokumentation

  • möglichst nah an den Code
  • erklären wie man das Objekt benutzt
  • Contracts: Seiteneffekte, Invarianten, Fehlerbehandlung, etc.
  • nicht die Implementierung!

meh:

def eigenvalues(M):
    """Wrapper around LAPACK's dggev subroutine.

    This function converts M into the form which LAPACK expects,
    invokes dgeev and returns the eigenvalues as computed from
    the parameters ALPHAR, ALPHAL and BETA. Depending on the
    output value of INFO, an exception is raised.
    """

besser:

def eigenvalues(M: Matrix) -> Vector:
    """ Calculate the eigenvalues of the matrix M.

    returns:
	Vector: all eigenvalues lambda_i of M, each repeated
	    according to its algebraic multiplicity.

    raises:
	ValueError: if the matrix is not diagonalizable
	NumericError: if the numerical algorithm does not
	    converge
    """

Kommentare

  • warum wird das so getan, nicht wie
  • nichts aus dem Code ersichtliches beschreiben
bool Thread::idle() {
  // If a child thread is present
  if (child)
    // call the idle action and return if has been processed
    { if (child->idle()) return true; }
  // The children have not processed the idle action,
  // process it now
  return proc_idle();
  }

Dokumentationsgeneratoren

  • als plain-text lesbar formatieren
def match_package_installed(self, package_name, zypper_output):
    """Check if a package was installed from zypper's output

    :param list package_list: list of all packages
    :param str log_line: zypper status line
    :returns: match or None if there isn't any match
    :rtype: match object, None
    """

besser lesbar, gleicher Inhalt:

def match_package_installed(
	self, package_name: List[str], zypper_output: str
    ) -> Optional[Match[str]]:
    """Check if a package was installed from zypper's output

    Args:
	package_list: list of all packages
	log_line: zypper status line

    Returns:
	regex match or None if there isn't any match
    """

Sphinx

sphinx-vanilla-project.png

Doxygen Doxygen_Xerces-C_Class_Index.png

Weitere Dokumentation

  • Prozesse & Code of Conduct
  • Infrastruktur

Tests

Program testing can be a very effective way to show the presence of bugs, but it is hopelessly inadequate for showing their absence.

– Edsger W. Dijkstra, The Humble Programmer (1972)

  • Bugfreiheit garantieren
  • Regressions verhindern
  • eigene API benutzen
  • Funktionalität verifizieren

Arten von Tests

  1. Unit Tests
  2. Integration Tests
  3. Performance Tests
  4. Mutation Tests

Unit Tests

  • Test des öffentlichen Interfaces einer Einheit
  • können viel setupcode & mocks erfordern
  • Gefahr: mini-Module für einfache Tests

Integration Tests

  • Test des gesamten Systems
  • pro: Test unter realistischen Bedingungen
  • con: Test unter realistischen Bedingungen

Welche Sorte Tests schreiben?

  • idealerweise beide
  • Integration Tests oft bei legacy code einfacher
  • für Bibliotheken: unit Tests
  • bei Benutzung externer Ressourcen: Integration Tests

Fehlerbehandlung

Fehler sind unvermeidbar und verursachen Komplexität.

void *malloc(size_t size);
int close(int fd);
const pid_t pid = fork();
if (pid == 0) { /* child process */ }
else {
  /* wait for the child and if it doesn't exit: */
  kill(pid, SIGKILL);
}

Fail early & fail hard

The undeniable FACT that people don't tend to check errors from close() should, for example, mean that delayed allocation must still track disk full conditions, for example. If your filesystem returns ENOSPC at close() rather than at write(), you just lost error coverage for disk full cases from 90% of all apps. It's that simple.

– Linus Torvalds

Fehler nachsichtig behandeln

  • nicht alles unvorhergesehene muss ein Fehler sein
>>> lst = [0, 1, 2, 3, 4]
>>> lst[0:2]
[0, 1]
>>> lst[0:42]
[0, 1, 2, 3, 4]
>>> lst[42:1]
[]

Variablen löschen:

set foo "bar"
delete foo # error if foo non-existent?
unset foo # no error
  • nicht überall angebracht
  • kann nach hinten losgehen:
$arr = [];
$arr[10] = "foo";
// count($arr) == ??
// $arr[0] == ??
// $arr[10] == ??

Fehler "schlucken"

const int write_err = write(fd, buf, nbytes);
if (write_err) {
  perror("Could not write data because: ");
  return;
}

Literatur und Quellen

  • Weniger schlecht programmieren, Kathrin Passig & Johannes Jander, 2013
  • A Philosophy of Software Design, John Ousterhout, 2018

Fragen?

Antworten!

Links

Legal

Pass through methods

class ThreadPool {
 private:
  queue_t threads;

 public:
  void add_thread_to_pool(const thread_t& thread) {
    this->threads.add(thread);
  }
  void remove_thread_from_pool(const thread_t& thread) {
    this->threads.remove(thread);
  }
}

Tutorials

Einsteiger abholen

  • einfach, prägnant, führt direkt zum Ziel
  • muss funktionieren
  • gibt Lust auf mehr

How to Guides

  • konkretes, fortgeschrittenes Problem lösen
  • verschiedene Wege zeigen

Erläuterung / Architektur

  • Warum?
  • Historischen Kontext

Assertions

assert(
  (strchr(type, ':') == NULL) &&
  (strchr(name, ':') == NULL) &&
  (strchr(action, ':') == NULL)
);
// cache_key must contain exactly two colons
sprintf(cache_key, "%s:%s:%s", type, name, action);

Exceptions vs Error Codes

const int fd = open(fname);
if (fd < 0) {
    // exception?
    throw io_error("Cannot open file", fname);
    // or error code?
    return ERR_CANNOT_OPEN_FILE;
}
// rest of function

Technical debt

Ich hab jetzt keine Zeit alles "richtig" zu machen…

– ich, viel zu oft

…und dann dauerte es am Ende dreimal so lange.

  • gutes Design kostet Zeit
  • technical debt muss irgendwann, mit Zinsen, zurückgezahlt werden