Forum: Ausbildung, Studium & Beruf Code Reviews


Announcement: there is an English version of this forum on EmbDev.net. Posts you create there will be displayed on Mikrocontroller.net and EmbDev.net.
von Dreister Frager (Gast)


Bewertung
-1 lesenswert
nicht lesenswert
Hallo zusammen,

haltet ihr Code Reviews, welche vom Autor selbst getätigt werden für 
sinnvoll?

MfG
Der Frager

von Jürgen W. (Firma: MED-EL GmbH) (wissenwasserj)


Bewertung
2 lesenswert
nicht lesenswert
Nein, das nennt man Programmieren - wenn's geht mit möglichst wenigen 
Fehlern.

von Ingo (Gast)


Bewertung
1 lesenswert
nicht lesenswert
Dreister Frager schrieb:
> Code Reviews, welche vom Autor selbst getätigt werden für sinnvoll?

So sinnvoll wie sich selbst die Haare zu schneiden... mit der Schere...

von Tom B. (botas)


Bewertung
2 lesenswert
nicht lesenswert
Wenn zwischen Schreiben und Review einige Jahre bis Jahrzehnte liegen 
kann es lustig sein. Spaß ist sinnvoll.

von Jens U. (urbiatorbi)


Bewertung
0 lesenswert
nicht lesenswert
Dreister Frager schrieb:
> haltet ihr Code Reviews, welche vom Autor selbst getätigt werden für
> sinnvoll?

Das ist bis zu dem Punkt sinnvoll, bis zu dem er alles, was er an 
Anforderungen zum Test bekommen hat, auch abarbeitet und nachweist, das 
es funktioniert. Das gilt auch für die Selbstgestellten Vorgaben.

Offen bleiben dabei zwei Dinge:

1) Was hat er alles ab Vorgaben zum Testen und Reviewen nicht gemacht, 
weil er es übersehen hat?

2) Was hat er alles nicht richtig gemacht und übersieht es beim 
optischen Inspizieren?


Beides ist kritisch. Das self review ist damit nur eine Teilabdeckung 
des reviews, sollte im review sheet auch entsprechend ersichtlich sein, 
z.B. durch punktweises Abhaken. Dann sieht jeder, was noch offen ist.

wir machen 4-Augen-Reviews, d.h. alles was einer beguckt hat, beschaut 
sich nochmal ein zweiter. Und zwar NICHT gleich zeitig!

von zitter_ned_aso (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Gab es hier nicht erst vor ein paar Wochen die gleiche Frage?

Beitrag #6262204 wurde von einem Moderator gelöscht.
von A. S. (achs)


Bewertung
1 lesenswert
nicht lesenswert
Ja, natürlich.

Das ist ähnlich wie alle Checklisten: gut, wenn es gemacht wird, besser 
wenn es ein anderer macht.

Die allermeisten Berichte oder Formalien sind doch genau dafür da, einen 
geordneten Abschluss zu bekommen. Sonst heißt es immer: dazu bin ich 
noch nicht gekommen.

von Kimonopython (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Dreister Frager schrieb:
> haltet ihr Code Reviews, welche vom Autor selbst getätigt werden für
> sinnvoll?

Nein, das sind keine 'Code reviews' sondern simpler 'Cargo Cult' - man 
ahmt etwas nach, dessen Sinn und Wirkungsweise man nicht verstanden hat.

Das fängt schon damit an, das man die 'roles' beim Review 
(Programmierer,Gutachter, Protokollant) nicht benannt hat und geht 
weiter über die Nicht-Bennung der Ein- und Ausgangsdokumente (Spec, 
Style-Code, Code, Änderungsbeschreibung, To-Do-Liste, Review-Protokoll).

Wahrscheinlich ist nur dem kleinsten Teil der Programmierer bewußt, das 
es für Reviews auch Standards wie IEEE-1028 und IEEE-730 gibt und man 
sich das genaue Procedere nicht aus den Finger saugen muss.

Oft kann aber der gemeine Coder nichts dafür, das die Reviews zur reinen 
Augenwischerei werden, es liegt bei dem Manager dafür zu sorgen, das der 
Review auch die Qualität des Produktes verbessert und das die Coder 
entsprechend geschult sind.

Wobei 'Schulung' schon mit der Ausfertigung einer
Checklist/Ablaufplan für den Review getan sein könnte - und diese 
Checklist/Pläne muß der Manager liefern. Oder die Schulung.

von Andreas S. (Firma: Schweigstill IT) (schweigstill) Benutzerseite


Bewertung
2 lesenswert
nicht lesenswert
Kimonopython schrieb:
> Das fängt schon damit an, das man die 'roles' beim Review
> (Programmierer,Gutachter, Protokollant) nicht benannt hat und geht
> weiter über die Nicht-Bennung der Ein- und Ausgangsdokumente (Spec,
> Style-Code, Code, Änderungsbeschreibung, To-Do-Liste, Review-Protokoll).

In typischen deutschen Großunternehmen (und auch kleineren Unternehmen 
mit vergleichbaren Strukturen) wird die Erstellung und Verwaltung 
solcher Dokumente dann aber zum Selbstzweck erhoben, so dass es dann gar 
nicht mehr um die entsprechenden Inhalte geht, sondern nur noch die 
Einhaltung der Formalien bei der Dokumentenerstellung geht.

Die damit einhergehenden Prozesse werden dann natürlich auch nicht mehr 
bezüglich ihrer Eignung für den eigentlichen Zweck bewertet, nämlich die 
Produktverbesserung, denn daran hängen doch etliche Pöstchen. Und da ja 
auch sehr viel Arbeit hineingeflossen ist, müsste sich ggf. auch 
derjenige, der den Prozess eingeführt hat, dafür verantworten, wenn die 
ganze Arbeit für die Tonne gewesen wäre.

Vor nicht allzu langer Zeit hatte ich von einem Kunden solch ein 
Konvolut an reinen Blabla-Texten bekommen. Dieses sollte als Vorlage für 
die konkreten Produktentwicklungen dienen. Auf meine sehr deutlichen 
Einwände, dass diese Dokumente völlig unbrauchbar wäre, hieß es nur, ein 
bestimmter Mitarbeiter hätte mehrere Jahre an diesen Vorlagen 
gearbeitet. Niemand kam damit zurecht. Ca. zwei Jahre später wurde der 
Autor dann in den Vorruhestand geschickt. Als ich bei dem letzten 
Projekt fragte, ob wieder diese Dokumente erstellt werden müsste, hieß 
es nur: "Nein, jetzt ist X. ja in Rente."

> Oft kann aber der gemeine Coder nichts dafür, das die Reviews zur reinen
> Augenwischerei werden, es liegt bei dem Manager dafür zu sorgen, das der
> Review auch die Qualität des Produktes verbessert und das die Coder
> entsprechend geschult sind.

Wenn es den überhaupt zu einem wie auch immer gearteten Review kommt, 
ist doch schon viel gewonnen. Wenn stattdessen nur Spezifikationen 
darüber verfasst werden, wie solch ein Review aussehen müsste, wenn man 
es denn durchführen würde, und das Review dann nur für diese Dokumente 
durchgeführt wird, aber niemals das Code Review stattfindet, hat man 
einfach nur Zeit verschwendet, die man lieber für die Entwicklung und 
nicht "nicht normgerechte" eigene Kontrollen verwendet hätte.

Bei einem anderen Kunden habe ich nämlich genau diese Situation erlebt. 
Ein Entwickler war monatelang damit beschäftigt, solche Dokumente 
insbesondere auch für meine Software zu erstellen, ohne auch nur einen 
Blick auf den Quelltext zu werfen. Irgendwann war er damit fertig, und 
ich fragte ihn ganz verwundert, wann denn das eigentliche Review 
stattfände. Er meinte daraufhin, dass es gar nicht mehr erforderlich 
sei, denn für die Zulassung des Produktes müsse nur die Vorgehensweise 
dokumentiert sein, aber nicht die eigentliche Durchführung erfolgen.

von klausi (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Andreas S. schrieb:
> Er meinte daraufhin, dass es gar nicht mehr erforderlich sei, denn für
> die Zulassung des Produktes müsse nur die Vorgehensweise dokumentiert
> sein, aber nicht die eigentliche Durchführung

Ha-ha! Schildbürgertum live! Entweder wird so etwas nicht verstanden und 
einfach für den Selbstzweck (oder Befriedigung) erstellt, wie du sagst, 
oder  man bedient damit irgendwelche Bürokraten und Verwalter in solchen 
Konzernen  oder anderen Unternehmen mit starren Strukturen...!

Deutschland begräbt sich unter ISO Normen, die halt einfach als purer 
Formalismus erfüllt werden.  Effektiv und inhaltlich bringts nix, gar 
nix. Hab das selbst schon oft erleben müssen.

Na dann, gute Nacht,  Deutschland!

von Kimonopython (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Andreas S. schrieb:
>> Oft kann aber der gemeine Coder nichts dafür, das die Reviews zur reinen
>> Augenwischerei werden, es liegt bei dem Manager dafür zu sorgen, das der
>> Review auch die Qualität des Produktes verbessert und das die Coder
>> entsprechend geschult sind.
>
> Wenn es den überhaupt zu einem wie auch immer gearteten Review kommt,
> ist doch schon viel gewonnen.

viel falsche Sicherheit ist gewonnen ...

Also statt 'wie immer geartet' dann 'mit absolut notwendigen Aufwand'

Und der wäre:
-Vorbereitung (Code und passende Doc/Spec aus-checken; comments im code 
ggf. überarbeiten, anlegen; log-files vom built bereithalten; ...)
-Durchführung (gemeinsames 'durchwandern' des Codes - 'was macht der 
Code hier' und 'warum')
-Email-protokoll mit Angabe wer hat welchen Code (tag/versionsnummer aus 
der versionsverwaltung, filenamen) nach welchen Kriterien (spec oder 
wenigstens kurze Funktionsbeschreibung) aus welchen beweggrund 
(Fehlerreport) durchgesehen.

Wenn es keinen konkreten Beweggrund gibt, wird auch kein Review 
durchgeführt. Ebenso wenn es keine Kriterien (bspw. 
style-guide,(Mini-)Spec) gibt. dann muss dieser Style-Guide eben 
erstellt werden. Oder man nimmt einen und passt den an. Man kann auch 
das Compiler-log verwenden in dem Sinne, alle warnings (natürlich bei 
-Wall) sind gelesen und verstanden.

Man kann auch dem Programmierer das Buch ISBN: 978-3-89721-567-2 geben 
und Seiten 61-86 zur Pflichtlektüre (während der bezahlten Arbeitszeit) 
erklären.

von Andreas S. (Firma: Schweigstill IT) (schweigstill) Benutzerseite


Bewertung
0 lesenswert
nicht lesenswert
Kimonopython schrieb:
> -Durchführung (gemeinsames 'durchwandern' des Codes - 'was macht der
> Code hier' und 'warum')

Das scheitert in vielen Fällen doch schon am "gemeinsam". Ein Entwickler 
bleibt auf sich gestellt. Derjenige, der als Kontrolleur infrage käme, 
ist durch bürokratische Formalismen gebunden. So ist die Realität in 
vielen Unternehmen.

> Wenn es keinen konkreten Beweggrund gibt, wird auch kein Review
> durchgeführt.

Wie schon geschrieben, kommt es vor, dass die Vorgehensweise 
dokumentiert wird, ohne anschließend dementsprechend vorzugehen.

> dann muss dieser Style-Guide eben
> erstellt werden. Oder man nimmt einen und passt den an.

Und dabei kommen dann eben ggf. diese völlig unbrauchbaren Dokumente und 
Vorlagen wie zuvor erwähnt heraus. Solche Dokumente werden ja häufig von 
Leuten verfasst, die entweder niemals selbst entwickelt oder sich als 
fachlich entbehrlich erwiesen haben. Hinzu kommt, dass solche Leute auch 
keine Projektverantwortung tragen, da sie ihre Projekte schon Jahre 
vorher verloren haben. Die erstellten Pamphlete passen dann weder zu den 
aktuellen Projekten, Entwicklungsprozessen, Werkzeugen, 
Programmiersprachen, "best practises". Wer sich daran hält, hat 
verloren.

Natürlich ist das nicht bei jedem Unternehmen so, aber bei der 
deutlichen Mehrzahl meiner Kunden erlebe ich genau dies in 
unterschiedlich starker Ausprägung. Und je größer und etablierter das 
Unternehmen ist, desto schlimmer ist es.

von Ingenieur24 (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Einen guten Code-Review kann nur ein Code-Review-Spezialist durchführen.
Nur er weiss, auf was er achten muss und wo die üblichen 
Nachlässigkeiten der Coder lauern. Er betrachtet das ganze Szenario aus 
einer anderen Perspektive. Natürlich muss der Code-Reviewer auch 
unabhängig sein, um ein bestmögliches Ergebnis sicherzustellen, also 
nicht mit dem Entwickler sozialisiert (Talk in der Kaffeeküche). Wir 
setzen deshalb für unsere Code-Reviews als auch für unsere Code-Audits 
nur externe Spezialisten ein. Diese sind schon für ein Stundenhonorar 
für 150 Euro buchbar. Natürlich fallen für die Code-Reviews deutlich 
weniger Stunden an als die Entwickler benötigen, wir kalkulieren meist 
mit 1:8 so dass die Kosten normalerweise nicht zu sehr ins Gewicht 
fallen.

von Kimonopython (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Andreas S. schrieb:
> Natürlich ist das nicht bei jedem Unternehmen so, aber bei der
> deutlichen Mehrzahl meiner Kunden erlebe ich genau dies in
> unterschiedlich starker Ausprägung. Und je größer und etablierter das
> Unternehmen ist, desto schlimmer ist es.

.. und mm schlimmsten ist es beim Grössten, bei Boeing: 
https://www.theguardian.com/us-news/2019/dec/11/boeing-737-max-plane-faa-regulators-crash-risk

>Das scheitert in vielen Fällen doch schon am "gemeinsam". Ein Entwickler
>bleibt auf sich gestellt. Derjenige, der als Kontrolleur infrage käme,
>ist durch bürokratische Formalismen gebunden.

Also wird der angelieferte Code einfach durchgewunken und nicht 
abgenommen
... aber solange gezahlt wird?!

Ja, ich weiß das ist nicht einfach. Da muß wohl jeder selbst einen Weg 
finden um mit sich und den Qualitätsansprüchen des Kunden seinen Frieden 
zu machen.

von Andreas (Gast)


Bewertung
1 lesenswert
nicht lesenswert
Also ich lese hier ein paar Dinge, die fuer mich sehr befremdlich 
klingen (aus der Perspektive in einer grossen US IT Firma):

Ingenieur24 schrieb:
> Einen guten Code-Review kann nur ein Code-Review-Spezialist durchführen.

Kimonopython schrieb:
> Email-protokoll
...
> Wenn es keinen konkreten Beweggrund gibt, wird auch kein Review
> durchgeführt.

Vielleicht sollte man erst mal die Begriffe klaeren, um sicherzustellen 
dass alle ueber das selbe reden?

- Unter "Code Review" verstehe ich ein "peer review", bei dem Kollegen 
die mit der Materie vertraut sind eine Aenderung ueberpruefen und 
absegnen bevor sie im zentralen Repository landet. Dazu werden Tools wie 
gerrit verwendet. Das ist nicht optional, die genauen Ablaeufe (noetige 
Anzahl der Approvals, wer kann approven) kann aber jedes Team fuer sich 
festlegen.
- Unter "Audit" verstehe ich eine anlassbezogene Pruefung auf bestimmte 
Aspekte (z.B. zur Sicherheitszertifizierung). Da wuerde dann ein 
Team-externer Pruefer beauftragt.

von A. S. (achs)


Bewertung
0 lesenswert
nicht lesenswert
Andreas S. schrieb:
> Solche Dokumente werden ja häufig von
> Leuten verfasst, die entweder niemals selbst entwickelt oder sich als
> fachlich entbehrlich erwiesen haben.

Das ist meist das eigentliche Problem. Und selbst wenn man Spezialisten 
heuert, wie hier

Ingenieur24 schrieb:
> Wir setzen deshalb für unsere Code-Reviews als auch für unsere Code-Audits
> nur externe Spezialisten ein.

macht das nur Sinn, wenn der Background der Leute zur Aufgabe und Umfang 
(nicht nur Sprache oder Plattform) passt und ein "unerfahrenes" Team 
diszipliniert und erzogen wird.

Am effektivsten ist ein Codereview natürlich intern, wenn ein 
gemeinsames Verständnis der Best-Practices entwickelt wird, und alle 
daran wachsen. Es muss nur sichergestellt sein, dass auch wirklich ein 
paar intelligente oder gute Leute dabei sind.

Beitrag #6263097 wurde von einem Moderator gelöscht.
von Jennifer R. (fernstudentin)


Bewertung
0 lesenswert
nicht lesenswert
Selbstkontrolle / entsprechende Tests gehören eh immer dazu. Sofern es 
sich nicht um ein 1-Mann-Projekt handelt, sollte ein Kollege das 
Code-Review übernehmen. Alles andere wäre irgendwie albern. Ist wie als 
wenn du ohne Musterlösung deine eigene Klausur kontrollierst.

von Andreas S. (Firma: Schweigstill IT) (schweigstill) Benutzerseite


Bewertung
1 lesenswert
nicht lesenswert
Jennifer R. schrieb:
> Alles andere wäre irgendwie albern. Ist wie als
> wenn du ohne Musterlösung deine eigene Klausur kontrollierst.

Willkommen in der Realität!

Um den Vergleich mit einer Klausur zu bemühen:
Wenn man von der Thematik keine Ahnung hat, reicht es aus, einen 
länglichen Text zu schreiben, in dem zunächst einmal möglichst abstrakt 
und unverbindlich festgelegt wird, was eine Klausur überhaupt ist. 
Hierfür werden möglichst ungebräuchliche und außerhalb des Kontextes 
dieses Textes unverständliche Bezeichnungen in der Langform und 
entsprechende Abkürzungen eingeführt. Dann beschreibt man unter 
exzessiver Anwendung dieser Begriffe, in welchen nachrangigen Dokumenten 
die Struktur der Klausur-Aufgabestellung und die Struktur der sich 
daraus ergebenden Anforderungsdokumente definiert werden. Dann wird 
beschrieben, in welchen nachrangigen Dokumenten das Vorgehen für die 
Erstellung der korrespondierenden Prüfspezifikationen auf Klausurebene 
beschrieben werden. Wichtig ist bis jetzt, jeglichen Bezug auf die 
eigentliche Klausur zu vermeiden. Das ganze wird dann rekursiv auf der 
Komponentenebene, die den Aufgabenblöcken entsprechen, wiederholt. Und 
dann noch einmal das ganze auf der Modulebene, die dann den Fragen bzw. 
Aufgaben entsprechen. Da das ganze etwas unübersichtlich geworden sein 
könnte, folgt nun ein Übersichtsdokument über die bisher erstellten 
Dokumente. Da es praktisch wäre, das Übersichtsdokument gleich am Anfang 
zu platzieren, wird eine Beschreibung zur Erstellung von 
Änderungsanweisungen erstellt. Dies wird auch gleich umgesetzt, nämlich 
in Form einer Änderungsanweisung für die Dokumentenreihenfolge. Leider 
ändern sich dadurch die vergebenen Dokumentennummern; das in diesem Fall 
anzuwendende Vorgehen folgt in der 
Dokumentennummernänderungsverfahrensbeschreibung festgelegt. Diese wird 
auch sofort angewendet; leider enthält sie einen kleinen Fehler und 
führt dazu, dass die Dokumentennummern völlig inkonsistent vergeben 
werden.

Spätestens an diesem Punkt hat der Korrektor keine weitere Zeit mehr, 
die Klausur zu korrigieren. Da bis dahin alles schlüssig erscheint, gibt 
es eine (in Schulnoten) fette 1. Auf Grund der fehlerhaften 
Dokumentennummern wird sie auf eine 1- abgewertet; wenn der Korrektor 
schlecht geschissen hat, auf eine 2+.

: Bearbeitet durch User
von Mal einwerfen (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Code review ...  oftmals waere es schon gut man koennte Konzepte 
besprechen um zu wissen ob man ueberhaupt auf dem richtigen Gleis ist. 
Der Code muss nicht wirklich angeschaut werden, solange der Zug in die 
richtige Richtung fahrt passt das schon.

von Realist (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Ab und an sind Code Reviews eine ganz gute Sache. Allerdings sollten 
dies 2 verschiedene Personen gegenseitig machen und darüber reden. Man 
kann dabei immer wieder was lernen. Allerdings ist dies allgmein viel zu 
Zeitaufwändig und auch nicht nötig, aber ab und an wenn etwas mehr Zeit 
vorhanden ist eine super sache...

von Qwertz (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Mal einwerfen schrieb:
> oftmals waere es schon gut man koennte Konzepte besprechen um zu wissen
> ob man ueberhaupt auf dem richtigen Gleis ist. Der Code muss nicht
> wirklich angeschaut werden, solange der Zug in die richtige Richtung
> fahrt passt das schon.

Klingt nach einem Design Workshop. Was spricht dagegen, sowas zu 
organisieren?

von Mal einwerfen (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Wenn die Leute denn verfuegbar waeren ... die machen alle was anderes

von Qwertz (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Mal einwerfen schrieb:
> Wenn die Leute denn verfuegbar waeren ... die machen alle was
> anderes

Dann schnapp sie dir!

von Nop (Gast)


Bewertung
3 lesenswert
nicht lesenswert
Bei vielen Projekten wär's ja schon ein Gewinn, wenn auch nur 
Compilerwarnungen aktiviert würden (z.B. GCC: -Wall -Wextra) und das 
Projekt verpflichtend ohne Warnungen durchcompilieren muß.

Antwort schreiben

Die Angabe einer E-Mail-Adresse ist freiwillig. Wenn Sie automatisch per E-Mail über Antworten auf Ihren Beitrag informiert werden möchten, melden Sie sich bitte an.

Wichtige Regeln - erst lesen, dann posten!

  • Groß- und Kleinschreibung verwenden
  • Längeren Sourcecode nicht im Text einfügen, sondern als Dateianhang

Formatierung (mehr Informationen...)

  • [c]C-Code[/c]
  • [avrasm]AVR-Assembler-Code[/avrasm]
  • [code]Code in anderen Sprachen, ASCII-Zeichnungen[/code]
  • [math]Formel in LaTeX-Syntax[/math]
  • [[Titel]] - Link zu Artikel
  • Verweis auf anderen Beitrag einfügen: Rechtsklick auf Beitragstitel,
    "Adresse kopieren", und in den Text einfügen




Bild automatisch verkleinern, falls nötig
Bitte das JPG-Format nur für Fotos und Scans verwenden!
Zeichnungen und Screenshots im PNG- oder
GIF-Format hochladen. Siehe Bildformate.

Mit dem Abschicken bestätigst du, die Nutzungsbedingungen anzuerkennen.