Code-Konventionen
code conventions
best practice
github
R
Diese Seite dient als Zusammenfassung unserer Beschlüsse hinsichtlich bestimmter Code-Konventionen, die wir im Team verfolgen wollen. Zum Absprechen dieser Themen treffen wir uns regelmäßig einmal im Monat.
Methodenteam
Umfang von Code-Abschnitten
- Generell sollte innerhalb einer Zeile möglichst wenig passieren. Als Orientierung: eine Zeile sollte nicht mehr als 100 Zeichen beinhalten.
- Gleiches gilt für eine Funktion: sie sollte maximal 100 Zeilen lang sein, und eher in Sub-Funktionen aufgeteilt werden.
- In R-Paketen sollte es pro exportierter Funktion ein eigenes Skript geben. Zu Hilsfunktionen haben wir noch nichts beschlossen.
- Bei Funktionen mit vielen Funktionsargumenten soll eine Liste von Funktionsargumenten innerhalb der internen Funktionsköpfe übergeben werden.
- Rigoroses Testen kann helfen bei Ergänzung oder Löschung von Funktionsargumenten den Überblick zu behalten.
Benennungskonventionen
- Bei längeren Objektnamen sollten Punkte vermieden werden und entweder CamelCases oder snake_cases genutzt werden. Das aber jeweils konsistent innerhalb von Paketen.
- Es sollten eher sprechende Namen als möglichst kurse Namen verwendet werden.
- Funktionsnamen sollten aus Verben bestehen.
- Objektnamen (die keine Funktionen sind) sollten Substantive sein.
- Dopplungen mit existierenden R-Objekten sollten vermieden werdeen.
- Bei der Objekt-Modifikation sollten Objekte nicht überschreiben werden, wenn sie sich substanziell ändern.
- Einige häufige Argumentnamen:
dat
,path/file/filePath/filename
,dir/folder
, … - Bei der Argumentreihenfolge sollte die Pipebarkeit und Output-Steuerung beachtet werden.
Dependencies
pkgnet kann genutzt werden, um die Dependencies innerhalb eines Pakets zu visualisieren.
Code Review
Eine übersicht zum GitHub-Workflow findet sich hier. Eine etwas ausführlicheres Tutorial zur Einführung in GitHub, aber auch zu Pull-Requests und Code-Reviews findet sich hier.
Vorgehen
- Person, die das Review anfordert, schlägt Personen für das Review for, in dem diese zur Pull-Request assigned werden.
- Sind mehrere Reviewer zum Review assigned, reicht es i.d.R. wenn eine:r das Review übernimmt (es sei denn, es wird explizit angefordert, dass alle das Review übernehmen)
- Übernimmt man ein Review, assigned man sich selbst den Pull-Request, dann wissen alle potenziell angesprochenen Reviewer gleich Bescheid, dass man übernimmt, auch wenn man noch nicht angefangen hat (bzw. noch keine Kommentare in den Code gesetzt hat)
Was muss bei der Anforderung eines Reviews beachtet werden?
- Eher kleinschrittige, abgeschlossene Bestandteile reviewen lassen.
- Priorität festlegen:
- high: innerhalb eines Tages
- medium: innerhalb einer Woche
- low: keine Zeitbegrenzung
Was sollen die Reviewer machen?
- Sich zum Review assignen, wenn man anfängt.
- Achten ob Benennungskonventionen und Formatkonventionen eingehalten wurden.
- Prüfen, ob der code verständlich geschrieben ist und möglichst keine erklärenden Kommentare benötigt.
- Tests auf Vollständigkeit überprüfen (sind mögliche Edge Cases (NA, negative Werte, hohe/niedrige Werte, anderer data type …) abgedeckt?).
- Mit den Tests rumspielen, versuchen die Funktion failen zu lassen.
Personelle Aufteilung
- eatPrepTBA: PF, NH, KAS, … (mal schauen)
- eatPrep: KAS, PF, NH
- eatPlot: NH, PF, KAS
- eatModel: SW, KAS, BB
- eatRep: SW, BB, KAS
- eatTools: SW, KAS, BB, NH, PF
- eatAnalysis: BB, SW, KAS
- eatGADS: BB, KAS, NH
- eatDB: BB, PF, NH
- eatFDZ: BB, NH
- eatCodebook: BB, SW, PF, NH
- eatRecode: BB, NH
- eatATA: BB, DD, ?
Kommunikation & Zeiterwartung
- Person, die das Review anfordert, schreibt ein paar Sätze dazu, worauf besonders geachtete werden sollte als Kommentar in den Pull-Request. Das können grobe Erwartungen an das Review sein (macht das konzeptuell Sinn, decken die Tests alle möglichen Edge cases ab …), oder eine Prioritätenfestlegung.
- Label sollen einheitlich über die Repositories erstell und genutzt werden (Bug/Enhancement …, Prio-Label)
Issues
- Wir nutzen GitHub-Projects um alle ToDos aus verschiedenen Repos an einem Ort zu sammeln.
- Wichtige Issues aus jedem Repo sollten im Projekt hinzugefügt werden.
Issues aus einem anderen Repository zum Projekt hinzufügen
- Click on
Add item
#beckerbenj/eatGADS
- From the suggested list you can choose the issue you want to add to the project, or create a new one.
- You can filter for all open issues with
is:issue is:open
.
Issue sorting
- Column “Done” contains all finished issues. They will be discussed one last time at the jour fix, and than the issue will be closed together.
- Everything should be an issue, not a draft! Drafts without a project can fit into iqb-research/to-dos.
Git-Benennungs-Konventionen
Branch-Benennung
Siehe hier.
Regeln:
- erstes Element: Feature/Bugfix/Hotfix/Docu/Release
- zweites Element: Issue-Nummer
- drittes Element: Kurz-Beschreibung
Beispiele:
feature/33-asNumericIfPossible-for-logicals
bugfix/45-dataframes-in-cleanifyStrings
docu/68-data-cleaning-vignette
Commit-Benennung
Siehe hier.
Regeln:
- erstes Element: worauf bezieht sich der Commit (zB Funktionsname)
- zweites Element: Kurz-Beschreibung
- in Kurzbeschreibung: Präsenz verwenden und sich kurz fassen
Beispiele:
- asNumericIfPossible: Add methods for logical and factor
- merge.GADSdat: Add by.x and by.y arguments
- cleanifyString: Add missing argument documentation
Wichtige Voraussetzung
Ein Commit muss eine in sich sinnvoll abgeschlossene Einheit darstellen!
FIAT Code-Konventionen
Umfang von Code-Abschnitten
- Generell sollte innerhalb einer Zeile möglichst wenig passieren. Als Orientierung gilt: Eine Zeile sollte maximal 100 Zeichen beinhalten. Dies gilt für Code, Dokumentation und Kommentare.
- Zu empfehlende Lösungsoptionen sind “harte” Zeilenumbrüche, das Erstellen und Aufrufen zusätzlicher Objekte und Pipen.
- Gleiches gilt für eine Funktion. Sie sollte maximal 100 Zeilen lang sein.
- Anzahl der Funktionsargumenten in einer Funktion
- Es gibt mehrere Möglichkeiten, mit sehr vielen Argumenten un Funktionskopf umzugehen:
- Funktion, die eine Liste mit Default-Werten erzeugt, die dann überschrieben werden können (z.B.
eatPlot::plot_lineplot
) - „…“
- Funktion, die eine Liste mit Default-Werten erzeugt, die dann überschrieben werden können (z.B.
- Ausschlaggebend dafür, ob Handlungsbedarf besteht, ist nicht die schiere Menge der Argumente, sondern wenn
- die Argumente sich stark in ihrer Hierarchie unterscheiden
- die Argumente viele Defaults haben
- Es gibt mehrere Möglichkeiten, mit sehr vielen Argumenten un Funktionskopf umzugehen:
Konventionen zum Schreiben von Funktionen
- In R-Paketen sollte es pro exportierter Funktion ein eigenes Skript geben.
- Eine Funktion sollte genau eine Sache tun, die aus dem Namen der Funktion hervorgeht, und davon Gebrauch machen, einzelne Aufgaben in andere (Hilfs-) Funktionen auszulagern (vgl. Umfang von Code-Abschnitten)
- Eine Ausnahme sind wrapper-Funktionen, die jedoch nichts weiter tun sollten, als die anderen Funktionen aufzurufen.
- Hilfsfunktionen werden im Skript der Funktion platziert, in dem sie verwendet werden.
- Falls Hilfsfunktionen von mehreren Haupt-/exportierten Funktionen verwendet werden, werden sie in
utils.R
verschoben. Diese Datei gibt es nur einmal pro Paket. - Falls der Umfang von
utils.R
zu stark anwächst, können Hilfsfunktionen gruppiert in separate Dateien geschoben werden.
- Funktionen sollten entweder keine side effects haben, oder ausschließlich side effects bewirken.
return()
oder keinreturn()
bleibt den Codenden überlassen- Es gibt Argumente, die dafür sprechen, konsequent am Ende der Funktion
return()
zu setzen (Eindeutigkeit, Nachvollziehbarkeit, Suchbarkeit) - aber auch dagegen. - Kriterium: Es sollte aus der Funktion heraus klar werden, was ausgegeben wird.
- Es gibt Argumente, die dafür sprechen, konsequent am Ende der Funktion
- Einrückungen/Indentation: Die Wahl zwischen
styleR
oder dem RStudio Default ist den Codenden überlassen. - Kommentare sollten a) möglichst gar nicht notwendig sein (weil der Code verständlich geschrieben ist) und b) das Warum und nicht das Was von Zeilen erklären.
if
-statements, loops und Funktionsdefinitionen (z.B. inlapply()
) sollten stets und auch dann umgebrochen werden, wenn sie nur einen Befehl enthalten, d.h. die Zeile endet mit{
, gefolgt von einem Zeilenumbruch.
Benennungskonventionen
- Bei längeren Objektnamen sollten Punkte vermieden werden und entweder CamelCases oder snake_cases genutzt werden. Das aber jeweils konsistent innerhalb von Paketen.
- Es sollten eher sprechende Namen als möglichst kurze Namen verwendet werden.
- Funktionsnamen sollten aus Verben bestehen.
- Objektnamen (die keine Funktionen sind) sollten Substantive sein.
- Dopplungen mit existierenden R-Objekten sollten vermieden werden.
- Bei der Objekt-Modifikation sollten Objekte nicht überschreiben werden, wenn sie sich substanziell ändern.
- Einige häufige Argumentnamen: dat, path/file/filePath/filename, dir/folder, … –> nächstes Mal besprechen
- Bei der Argumentreihenfolge sollte die Pipebarkeit und Output-Steuerung beachtet werden.
Konventionen für Pakete
- Rigoroses Testen
- Rigoroses Testen kann helfen, bei Ergänzung oder Löschung von Funktionsargumenten den Überblick zu behalten.
- Umsetzung mit
testthat
- Es wird eine Coverage > 90% angestrebt
- Die Anzahl von Dependencies sollte gering gehalten werden.
- pkgnet kann genutzt werden, um die Dependencies innerhalb eines Pakets zu visualisieren.
- Insbesondere sollten “teure” Dependencies, die ihrerseits viele Dependencies aufrufen, vermieden werden.
- Ggf. sollten Funktionen selbst (nach-) gebaut werden.
Offen –> Besprechung am 6.12. oder 13.12.
- Kristophs Konventionen zur Benennung von Objekten („df_“, „pt_“, …)
- Scope von Paketen
- Code Review (auf Basis von https://iqb-research.github.io/IQB-Methods/docs/discussions/code_conventions.html)
- commit-messages
- Benennung von feature branches
Mögliche nächste Themen
- Indentierung?
- Häufige Argumentnamen
- Dplyr und/oder pipen?
- Kommentar- & Austauschsprache
- “Sprechender Code”? Vgl. best Coding Practices…
- finales Ziel: styleR?
- Tests (Abdeckung, Umsetzung, Style, …)
- schematische Paket-Dokumentationen (nächstes Thema!)
- potentiell interessantes R Paket zur Visualisierung: https://uptake.github.io/pkgnet/articles/pkgnet-intro.html
- Paketstruktur-Vignette/Dokumentation auf Quarto-Homepage
- Labels bei Pull-Requests und Issues
- Umgang mit failed Tests bei Pull-Requests (ggf. differenziert nach Stumi/Wimi)
- Workflow Update Pull Requests (Wiedervorlage nach Einarbeitung von requested Changes)