# Review
###### tags: `Praktikum`

Rollen, die wir verteilen sollen:
- Moderator
- Reviewer
- Protokollant


Code Review Checklist – Java WP-CT WiSe21

ToDo: mark each item with
- X if condition is valid
- NA if condition is not applicable
- U if conditions is not fulfilled
- List any deviation by stating file and lines of the finding

| 1. Specification / Design | Finding | Location (File(s), Line(s)) |
|---------------------------------------------------------------------------------------------------------------------------|---------|-----------------------------|
| a. Is the functionality described in the specification fully implemented by the code? | U | CSV processing missing |
| b. Is there any excess functionality in the code but not described in the specification? | X | pruefeWahrheit method (Bedingungsueberdeckung.java:163) |
| | | |
| 2. Initialization and Declarations | | |
| a. Are all local and global variables initialized before use? | | |
| b. Are variables and class members of the correct type and are the using appropriate modifiers? | | |
| c. Are variables declared in the proper scope? | | |
| d. Is a constructor called when a new object is desired? | | |
| e. Are Variable names spelled correctly and consistently | | |
| f. Make sure that primitive data types are not set to null or empty | | |
| g. Is 'static' keyword used correctly? | | |
| h. Is every variable correctly typed? | | |
| i. Are there variables that should be constants? | | |
| | | |
| 3. Method Definition | | |
| a. Are descriptive method names used in accord with the naming convention? | U | Bedingungsueberdeckung line 243(Pack muss groß) |
| b. Is every method parameter value checked before being used? | U | Bedingungsueberdeckung line 27,119,139,161,172,205,243 |
| c. Are there static methods that should be non-static or vice-versa? | X | |
| | | |
| 4. Class Definition | | |
| a. Does each class have an appropriate constructor? | | |
| | | |
| 5. Data References | | |
| a. For every object or array reference: is the value certain to be non-null? | X | |
| | | |
| 6. Method Calls | | |
| a. Are parameters presented in the correct order? | | |
| b. Are parameters of the proper type for the method being called? | | |
| c. Are method return values used properly? Cast to the required type? | | |
| d. When calling a method that has a return value, be sure to use the return value properly. | | |
| | | |
| 7. Arrays | | |
| a. Are there any off-by-one errors in array indexing? | X | |
| b. Can array indexes ever go out-of-bounds? | X | |
| c. Are array declarations syntactically correct? | X | |
| d. Are row and column indexed in the right order for a 2D array? | X | |
| | | |
| 8. Object Comparison | | |
| a. Are all objects (including Strings) compared with "equals" and not "=="? | | |
| | | |
| 9. Output Format | | |
| a. Are there any spelling or grammatical errors in displayed output? | U | "Coverage" sometimes in caps sometimes not (Main.java:22), missing dot (Main.java:34), Sometimes single sometimes double quotes (Main.java:31) |
| b. Is the output formatted correctly in terms of line stepping and spacing? | X | |
| | | |
| 10. Computation, Comparison and Assignment | | |
| a. Check order of computation/evaluation, operator precedence and parenthesizing | | |
| b. Can the denominator of a division ever be zero? | | |
| c. Is integer arithmetic, especially division, ever used inappropriately, causing unexpected truncation/rounding? | | |
| d. Check each condition to be sure the proper relational and logical operators are used. | | |
| e. If the test is an error-check, can the error condition actually be legitimate in some cases? | | |
| f. Does the code rely on any implicit type conversions? | | |
| g. Are there any computations with mixed data types? | | |
| h. Is overflow/underflow possible during computation? | | |
| i. For each expression with more than one operator: are the assumptions about order of evaluation and precedence correct? | | |
| j. Are there any comparisons between variables of inconsistent types? | | |
| k. Are the comparison operators correct? | | |
| | | |
| 11. Exceptions | | |
| a. Are all relevant exceptions caught? | | |
| b. Is the appropriate action taken for each catch block? | | |
| | | |
| 12. Flow of Control | | |
| a. Check that nested if statements don't have “dangling else” problems. | | |
| b. Are all loops correctly formed, with the appropriate initialization, increment and termination expressions? | | |
| c. Are open-close parentheses and brace pairs properly situated and matched? | | |
| d. Do logical expressions evaluate to the correct true or false value? | | |
| e. Do Boolean functions return the correct value? | | |
| f. Has each boolean expression been simplified by driving negations inward? | | |
| g. For every boolean test: Is the correct condition checked? | | |
| h. Is each Boolean expression correct? | | |
| i. Are there improper and unnoticed side-effects of a comparison | | |
| j. Has an "&" inadvertently been interchanged with a "&&" or a "\|" for a "\|\|"? | | |
| k. Does the code avoid comparing floating-point numbers for equality (IEEE 754)? | | |
| l. Is every three-way branch (less,equal,greater) covered? | | |
| m. For each loop: Is the best choice of looping constructs used? | | |
| n. Will all loops terminate? | | |
| o. When there are multiple exits from a loop: is each exit necessary and handled properly? | | |
| p. In a switch statement is every case terminated by break or return? | | |
| q. Do all switch statements have a default branch? | | |
| r. Are missing switch case break statements correct and marked with a comment? | | |
| s. Is the nesting of loops and branches too deep, and is it correct? | | |
| t. Can any nested if statements be converted into a switch statement? | | |
| u. Are null bodied control structures correct and marked with braces or comments? | | |
| v. Does every method always terminate? | | |
| w. Do named break statements send control to the right place? | | |

zu deutsch
| 1. Spezifikation / Design Clemens | gefunden | Ort (Datei(en)/Zeile(n)) |
|-----------------------------------------------------------------------------------------------------------------------------------------|----------|--------------------------|
| a. Ist die in der Spezifikation beschriebene Funktionalität vollständig durch den Code implementiert? | | |
| b. Gibt es eine überschüssige Funktionalität im Code, die jedoch nicht in der Spezifikation beschrieben ist? | | |
| | | |
| **2. Initialisierung und Erklärungen** | | |
| a. Werden alle lokalen und globalen Variablen vor der Verwendung initialisiert? | | |
| b. Sind Variablen und Klassenmitglieder vom richtigen Typ und verwenden sie geeignete Modifikatoren? | | |
| c. Werden Variablen im richtigen Bereich deklariert? | | |
| d. Wird ein Konstruktor aufgerufen, wenn ein neues Objekt gewünscht wird? | | |
| e. Sind Variablennamen richtig und konsistent geschrieben? | | |
| f. Stellen Sie sicher, dass primitive Datentypen nicht auf null oder leer gesetzt sind | | |
| g. Wird das Schlüsselwort 'static' korrekt verwendet? | | |
| h. Ist jede Variable richtig eingegeben? | | |
| i. Gibt es Variablen, die Konstanten sein sollten? | | |
| | | |
| **3. Methodendefinition** | | |
| a. Werden beschreibende Methodennamen gemäß der Namenskonvention verwendet? | | |
| b. Wird jeder Methodenparameterwert vor seiner Verwendung überprüft? | | |
| c. Gibt es statische Methoden, die nicht statisch sein sollten oder umgekehrt? | | |
| | | |
| **4. Klassendefinition** | | |
| a. Hat jede Klasse einen geeigneten Konstruktor? | | |
| | | |
| **5. Datumsangaben** Clemens | | |
| a. Für jede Objekt- oder Arrayreferenz: Ist der Wert sicher nicht Null? | | |
| | | |
| **6. Methodenaufrufe** | | |
| a. Werden Parameter in der richtigen richtigen Darstellung? | | |
| b. Sind die Parameter vom richtigen Typ für die Methode verbessertufen? | | |
| c. Werden Methoden-Rückgabewerte richtig verwendet? Gießen auf den falschen Typ? | | |
| d. Stellen Sie beim Aufrufen einer Methode mit einem Rückgabewert sicher, dass der Rückgabewert wird verwendet wird. | | |
| | | |
| **7. Arrays** | | |
| a. Gibt es Fehler bei der Array-Indizierung? | | |
| b. Können Array-Indizes warten, der der Grenzen liegt? | | |
| c. Sind Array-Deklarationen syntaktisch korrigiert? | | |
| d. Werden Zeilen und Spalten in der richtigen Reihenfolge für ein 2D-Array indiziert? | | |
| | | |
| **8. Objektvergleich** | | |
| a. Werden alle Objekte (mit "gleich" und nicht mit "==" verglichen? | | |
| | | |
| **9. Ausgabeformat** Clemens | | |
| a. Gibt es Rechtschreib- oder Grammatikfehler in der angezeigten Ausgabe? | | |
| b. Ist die Ausgabe korrekt in Form von Zeilenschritt und Abstand formatiert? | | |
| | | |
| **10. Rechnung, Vergleich und Zuordnung** | | |
| a. Die Sie des Verlusts der Auswertung, der Strafen des Operators und der Klammerung | | |
| b. Kann der Nenner einer Division verloren Null sein? | | |
| c. Wird die Ganzzahlarithmetik, die Teil der Verwaltung, die Verwaltung, die Berechtigung, die Zuzahleten Kürzungen? | | |
| d. Wie Sie jede mögliche, um welche, dass die richtigen relationalen und logischen Operatoren verwendet werden. | | |
| e. Wenn der Test eine Fehlerprüfung ist, kann die Fehlerbedingung in einer bestimmten Berechtigung legitim sein? | | |
| f. Beruht der Code auf impliziten Typkonvertierungen? | | |
| G. Gibt es solche mit gemischten Datentypen? | | |
| h. Ist ein Überlauf möglich? | | |
| ich. Für jeden Ausdruck mit mehr als einem Betreiber: Sind die Informationen über die Angaben der Bewertung und die richtige Korrektur? | | |
| j. Gibt es welche zwischen Variablen inkonsistenter Typen? | | |
| k. Sind die Vergleichsoperatoren korrekt? | | |
| | | |
| **11. Ausnahmen** | | |
| a. Werden alle relevanten Ausnahmen abgefangen? | | |
| b. Wird für jeden Fangblock die entsprechende Maßnahme ergriffen? | | |
| | | |
| **12. Kontrollfluss** | | |
| a. Überprüfen Sie, ob verschachtelte Anweisungen keine Probleme mit "Dangling else" haben. | | |
| b. Sind alle Schleifen korrekt mit den entsprechenden Initialisierungs-, Inkrement- und Beendigungsausdrücken gebildet? | | |
| c. Sind Open-Close-Klammern und Klammerpaare richtig positioniert und aufeinander abgestimmt? | | |
| d. Werden logische Ausdrücke auf den richtigen wahren oder falschen Wert ausgewertet? | | |
| e. Geben boolesche Funktionen den richtigen Wert zurück? | | |
| f. Wurde jeder boolesche Ausdruck vereinfacht, indem Negationen nach innen getrieben wurden? | | |
| G. Für jeden Booleschen Test: Wird der korrekte Zustand überprüft? | | |
| h. Ist jeder Boolesche Ausdruck korrekt? | | |
| ich. Gibt es unangemessene und unbemerkte Nebenwirkungen eines Vergleichs? | | |
| j. Wurde versehentlich ein "&" gegen ein "&&" oder ein "\|" für ein "\|\|" ausgetauscht? | | |
| k. Vermeidet der Code den Vergleich von Gleitkommazahlen auf Gleichheit (IEEE 754)? | | |
| l. Ist jeder Dreiwegezweig (kleiner, gleich, größer) abgedeckt? | | |
| m. Für jede Schleife: Wird die beste Auswahl an Schleifenkonstrukten verwendet? | | |
| n. Werden alle Schleifen beendet? | | |
| Ö. Wenn es mehrere Ausgänge aus einer Schleife gibt: Ist jeder Ausgang notwendig und wird ordnungsgemäß behandelt? | | |
| p. Wird in einer switch-Anweisung jeder Fall durch break oder return beendet? | | |
| q. Haben alle switch-Anweisungen einen Standardzweig? | | |
| r. Sind fehlende Switch Case Break-Anweisungen korrekt und mit einem Kommentar gekennzeichnet? | | |
| s. Ist das Verschachteln von Schleifen und Zweigen zu tief und ist es richtig? | | |
| t. Können verschachtelte if-Anweisungen in eine switch-Anweisung konvertiert werden? | | |
| u. Sind Kontrollstrukturen mit Nullkörper korrekt und mit geschweiften Klammern oder Kommentaren gekennzeichnet? | | |
| v. Wird jede Methode immer beendet? | | |
| w. Senden benannte break-Anweisungen die Kontrolle an den richtigen Ort? | | |

Clemens 1 5 9
Eickholt 2 6 10
Jedermann 3 7 11
Königsfeld 4 8 12

Die 5 Prinzipien für objektorientiertes Softwaredesign:
SOLID
S = Single Responsibility
O = Open Close Principle
L = Liskov Substitution Principle
I = Interface Segregation Principle
D = Dependency Inversion Principle

Software sollte so Komplex wie nötig aber auch einfach wie möglich sein.
In diesem Fall haben wir 1 Klasse, die einliest, die unterschiedlichen Algorthmen auf das eingelesene anwendet und auch noch das alles wieder ausgibt!

Dadurch ist die **Single Responsibility** nicht gegeben.
Als Faustregel kann man sagen: Wenn man erklärt, was eine Klasse macht und dort das Wort *und* oder *oder* bei benutzt, sollte man überlegen, diese Klasse weiter aufzudröseln.
Es sollte nur 1 Grund geben, warum man eine Klasse anfasst und eine Klasse sollte nur eine Verantwortung haben!
Dadurch ist z.B. auch einfacher ein Projekt zu warten und auch um die Seiteneffekte zu minimieren.
Zusätzlich wird es erleichtert, wenn man mit verschiedenen Entwickler an einer Klasse arbeitet, da dann ein Entwickler die Verantwortung über eine Klasse übernehmen kann.

Verbesserungsvorschlag:
Je Teilbereich Einlesen, Algorithmus anwenden und schreiben jeweils einen eigenen Bereich erstellen (Ordnerstruktur oder Klasse)

Ebenso ist das OpenClose Principle nicht gegeben
In diesem Code wurde z.b. der coveragetype

Im Optimalfall definieren wir z.B. via Interface, wie ein Algorithmus auszusehen hat (welche Variablen/Methoden umgesetzt werden sollen) Anschließend kann man nun recht einfach diese Algorithmen in jeweils einer Klasse, die von diesem Interface vererbt bekommt, die Umsetzung implementieren.
Dadurch sind wird geschlossen für Veränderungen (der jeweiligen Klasse und somit schon umgesetzten Algorithmen) und dennoch offen für Erweiterungen (hinzufügen weiterer Algorithmen, die jeweils das Interface umsetzen müssen)

In der Main wird ein Objekt Bedinungsüberdeckung