Code Review: Unterschied zwischen den Versionen
Aus Das Sopra Wiki
K Änderungen von Langenfeld (Diskussion) wurden auf die letzte Version von Dietsch zurückgesetzt Markierung: Zurücksetzung |
|||
| (12 dazwischenliegende Versionen von 2 Benutzern werden nicht angezeigt) | |||
| Zeile 48: | Zeile 48: | ||
Ein gutes Beispiel aus dem Softwarepraktikum ist: | Ein gutes Beispiel aus dem Softwarepraktikum ist: | ||
<syntaxhighlight lang=" | <syntaxhighlight lang="md">94cb043 see #27: Wayfinding improved to support bridges</syntaxhighlight > | ||
Ein schlechtes Beispiel ist: | Ein schlechtes Beispiel ist: | ||
<syntaxhighlight lang=" | <syntaxhighlight lang="md">959f313 Woololoo</syntaxhighlight> | ||
====Naming (und Coding Conventions)==== | ====Naming (und Coding Conventions)==== | ||
| Zeile 78: | Zeile 78: | ||
*Sind Konstanten in entsprechenden Variablen verpackt und passend benannt? | *Sind Konstanten in entsprechenden Variablen verpackt und passend benannt? | ||
*Alle Benennungen sind frei von Füllwörtern, Artikeln, dummen Witzen, etc. | *Alle Benennungen sind frei von Füllwörtern, Artikeln, dummen Witzen, etc. | ||
*Benennungen sind einheitlich, d.h. gleiche Dinge heißen immer gleich z.b < | *Benennungen sind einheitlich, d.h. gleiche Dinge heißen immer gleich z.b <syntaxhighlight lang="csharp" inline>Get...</syntaxhighlight>, <syntaxhighlight lang="csharp" inline>Set...</syntaxhighlight> usw. | ||
*Klassen haben Namen im Singular z.B. | *Klassen haben Namen im Singular z.B. <syntaxhighlight lang="csharp" inline>class SpaceShip</syntaxhighlight> | ||
*Instanzen von Listen, Mengen, etc. sind im Plural benannt z.B. | *Instanzen von Listen, Mengen, etc. sind im Plural benannt z.B. <syntaxhighlight lang="csharp" inline>private List<SpaceShip> mSpaceShips;</syntaxhighlight > | ||
*Methoden sind mit Verb und Substantiv benannt <syntaxhighlight lang="csharp" inline>public GetEnemySpaceShips()</syntaxhighlight > | *Methoden sind mit Verb und Substantiv benannt <syntaxhighlight lang="csharp" inline>public GetEnemySpaceShips()</syntaxhighlight > | ||
| Zeile 89: | Zeile 89: | ||
====Keine offensichtlichen Fehler==== | ====Keine offensichtlichen Fehler==== | ||
Versuchen Sie den Code zu verstehen, und Fälle zu finden, in denen der Code nicht tut was er soll. Manchmal hat der Autor einfach einen Grenzfall nicht bedacht, oder eine Annahme gemacht, die nicht zutrifft, und bei der der Code versagt. | |||
====Auf eine sinnvolle Weise==== | ====Auf eine sinnvolle Weise==== | ||
Ab und an kommt es vor, dass der Code zwar das korrekte Ergebnis liefert, aber dies auf eine nicht der Aufgabe angemessene Art und Weise. | Ab und an kommt es vor, dass der Code zwar das korrekte Ergebnis liefert, aber dies auf eine nicht der Aufgabe angemessene Art und Weise. Versuchen Sie solche Stellen im Code zu finden. | ||
Der folgende Code macht das Richtige: Er prüft ob es ein vorhergehendes Element in der Liste der <code>Results</code> gibt. Er verwendet dafür aber Exceptions, die sehr langsam sind, den Code unnötig schwierig zu lesen machen, und für diese Aufgabe nicht angemessen sind, da wir die Größe der Liste | Der folgende Code macht das Richtige: Er prüft ob es ein vorhergehendes Element in der Liste der <code>Results</code> gibt. Er verwendet dafür aber Exceptions, die sehr langsam sind, den Code unnötig schwierig zu lesen machen, und für diese Aufgabe nicht angemessen sind, da wir die Größe der Liste kennen. | ||
< | <syntaxhighlight lang="csharp"> | ||
protected bool PreviousResultExists() | |||
{ | { | ||
try | try | ||
{ | { | ||
int resultId = Results[SelectedResultId - 1]; | int resultId = Results[SelectedResultId - 1]; | ||
| Zeile 109: | Zeile 109: | ||
return false; | return false; | ||
} | } | ||
}</ | } | ||
</syntaxhighlight> | |||
Verbesserungsvorschlag: | Verbesserungsvorschlag: | ||
< | <syntaxhighlight lang="csharp"> | ||
SelectedResultId > 0;</ | protected bool PreviousResultExists() => | ||
SelectedResultId > 0; | |||
</syntaxhighlight> | |||
====Code vereinfachen==== | ====Code vereinfachen==== | ||
Es kann sein, dass der Code das Richtige auf angemessene Art und Weise macht, aber trotzdem einfacher zu lesen sein könnte. | Es kann sein, dass der Code das Richtige auf angemessene Art und Weise macht, aber trotzdem einfacher zu lesen sein könnte. | ||
Folgendes Beispiel | |||
< | <syntaxhighlight lang="csharp"> | ||
public T GetComponent<T>() | |||
{ | { | ||
foreach (var c in mComponents) | foreach (var c in mComponents) | ||
| Zeile 130: | Zeile 135: | ||
} | } | ||
return default(T); | return default(T); | ||
}</ | } | ||
</syntaxhighlight> | |||
lässt sich z.B. einfacher mit einer LINQ-Expression und einem besseren Methodennamen ausdrücken. | lässt sich z.B. einfacher mit einer LINQ-Expression und einem besseren Methodennamen ausdrücken. | ||
< | <syntaxhighlight lang="csharp"> | ||
public T GetFirstComponent<T>() | |||
{ | { | ||
return mComponents.OfType<T>().First(); | return mComponents.OfType<T>().First(); | ||
}</ | } | ||
</syntaxhighlight> | |||
===Architektur=== | ===Architektur=== | ||
Prüfen Sie ob der Code den [https://sopranium.de/CleanCode#Prinzipien architektonischen Grundprinzipien] entspricht. | |||
---- | ---- | ||
*[https://google.github.io/eng-practices/review/reviewer/standard.html The Standard for Code Review] | *[https://google.github.io/eng-practices/review/reviewer/standard.html The Standard for Code Review] | ||
*[https://google.github.io/eng-practices/review/reviewer/looking-for.html What to look for in a Code Review] | *[https://google.github.io/eng-practices/review/reviewer/looking-for.html What to look for in a Code Review] | ||
*[https://github.com/google/eng-practices Google Engineering Principles Documentation] | *[https://github.com/google/eng-practices Google Engineering Principles Documentation] | ||
