Wie steht ihr zu diesen Clean-Code-Empfehlungen?

Hallo "Kollegen",

ich habe mir das Buch Clean Code: A Handbook of Agile Software Craftsmanship "reingezogen" und fand es sehr hilfreich. Trotzdem werden in diesem Buch einige Standpunkte, bzw. Vorgehensweisen vertreten, die ich so nicht unterstützen kann. Diese möchte ich aufzählen und begründen.

Ich würde mich freuen, wenn ich von erfahrenen Entwicklern diesbezüglich ein kleines Feedback bekommen könnte, so dass ich ggf. meine Ansichten revidieren kann (oder muss).

1. Rückgabe von Object, das nie null ist, sondern "leere" Instanz
   Beispiel:

public IList<String> GetList(bool bla) {
  List<String> back = new List<Sting>();

    if(bla) {
      // ... bla blub do something
   }
 
  return back;
}

Es wird ausdrücklich darauf hingewiesen, dass leere Instanzen erstellt werden sollen und keine Null-Referenzen.
Ich würde obigen Code nie so reinstellen, sondern null zurückgeben, wenn bla nicht true ist, und back würde ich mit null deklarieren.
Was spricht dagegen? Wird bei dieser Vorgehensweise nicht auch Speicherplatz gespart? OK, auf der anderen Seite muss natürlich immer auf null geprüft werden, nach dem Aufruf, das sollte aber imo immer der Fall sein, das setzt gute Programmierung voraus.

2. Interfacenamen beginnen in einer Factory nicht mit dem "herkömmlichen" I, da es, so der Autor, der Benutzer nicht wissen muss ob es sich um ein Interface handelt, da dies ablenken könnte. Nun, ich benutze im Namen von Interfaces (ob Factory oder nicht) immer mit vorangestelltem I.

3. Es werden bei einzeiligen Anweisungen, z.B. nach einer Schleife oder einer Bedingung keine Klammern gesetzt

Beispielcode:

  while(true)
    if(true)
      doSomething();
  if(true)
    doMore();

Ich finde das keinen schönen Stil. OK, ich muss mehr schreiben (Klammern), jedoch denke ich nicht, dass dies ins Gewicht fällt. Ich würde immer Klammern verwenden. Obiger Code würde dann so aussehen:

  while(true) {
    if(true) {
      doSomething();
    }
  }

  if(true) {
    doMore();
  }

4. Wird ein Objekt übergeben (by Reference) und dieses manipuliert, dann soll dieses explizit zurückgegeben werden.

Beispielcode:

public StringBuffer ManipBuffer(StringBuffer inBuffer) { ...

Was spricht denn hier für diese Vorgehensweise? Ich bin hier folgendes gewohnt:

public void ManipBuffer(StringBuffer inBuffer) { ...

5. Ein heikles Thema, Klammerung. Ich habe anfänglich die Klammerung auf gleicher Spaltenebene (also untereinander) gesetzt (und lieben gelernt). Dann kam ich auf ein Projekt, wo die Klammerung so verwendet wurde, dass die öffnende Klammer in der gleichen Zeile gesetzt wird. Ich habe das dann so verinnertlicht, dass ich bei dem übernächsten Projekt mir Klammern untereinander nicht mehr aneignen wollte - das bläht den Code nur auf und entgegen vielen Behauptungen ist der Code nicht übersichtlicher. Wie macht ihr die Klammerung und warum ... man kann ja Gott sei Dank mit eigenen Formattern die IDE so einstellen, dass die Klammern nach Wunsch gesetzt werden.

6. Der Autor sagt, dass nicht immer ein einziges Return-Statement Sinn macht, z.B. bei kleinen überschaubaren Methoden,

Beispielcode:

public int GetCode(bool blub) {
  int ret = int.MinValue;

  if(blub) {
    return 1;
  }
  else {
    return 2;
  }  
}

Ich würde trotzdem immer nur einen Ausgang aus einer Methode bevorzugen, da bin ich konsequent.

7. Zugriffe auf Variablen. Ich sehe immer wieder, dass "einfach nur" der Variablenname (ob nun Instanzvariable, Klassenvariable oder lokale Variable) - letzters macht ja Sinn, geht nicht anders, beim Zugriff verwendet wird.

Ich habe mir folgendes angewöhnt:

Bei Instanzvariablen immer den this oder base-Operator voranstellen, dann weiß man sofort, um welchen Typ (und wo sie beheimatet ist) es sich bei der Variablen handelt.

Bei Klassenvariablen schreibe ich immer die Klasse davor (auch wenn sich die Klassenvariablen in der gleichen Klasse befindet, um so eine Abgrenzung von lokalen Variablen sichtbar zu machen).

8.) Horizontale Ausrichtung

Ich sehe immer wieder folgende Variablendeklarationen:

private   Socket         socket;
protected String         output;
protected FitnessContext fitnessContext = null;

Das kann ja für den ein oder anderen übersichtlich aussehen, jedoch macht es bei Änderrungen einen großen Aufwand, wenn ein Variablenname länger ist als der bisher längste, denn dann müssen alle engepasst werden, somit bleibe ich bei folgenden Deklarationen:

private Socket socket;
protected String output;
protected FitnessContext fitnessContext = null;

9. Mehrfachaufrufe in einer Zeile

Es wird empfohlen, aus folgendem Aufruf:

final String outputDir = ctx.getOptions().getScratchDir().getAbsolutePath();

folgenden zu machen:

Options opts = ctx.getOptions();
File scatchDir = opts.getScratchDir();
final String outputDir = scratchDir.getAbsolutePath();

Nun denke ich schon, dass dies Sinn macht, jedoch benutze ich erste Zeile immer dann, wenn ich auf jeden Fall weiß, dass es nicht krachen kann (NullReferenceException), dann kann man schon mal alles in eine Zeile packen.

10. Komplexe Bedingungen sollten in eine Methode ausgelagert werden.

Beispielcode:

  if(!bla && gedoens && i > 5) { ...

soll ausgelagert werden zu ...

  public bool IsAccess(bla, gedoens, i) {
    return !bla && gedoens && i > 5;
  }

OK, die Parameter müssen natürlich nicht übergeben werden, wenn es sich um Instanzvariablen handelt, jedoch finde ich die Vorgehensweise nur dann Sinnvoll, wenn diese Abfrage öfters als einmal verwendet wirde - ansonsten würde ich hier keine neue Methode schreiben.

Unter dem Strich fand ich das Buch natürlich sehr hilfreich. (In der Regel kann man immer was von jemand lernen, der jahrzehntelang programmiert hat UND, jetzt kommt das Wichtige, einigermaßen mit der Zeit geht 😄)

OK, das war es erst mal. Aufruf an alle "Nitpicker" wird nun gestartet! 😉

Danke schon mal für die Teilnahme an der Diskussion im Voraus.

P.S. Ihr müsst natürlich nicht jeden Punkt kommentieren.

1:
wenn du in dem fall eine konkrete leere instanz zurückgibst, kannst du das ergebniss sofort in einer foreach nutzen.

2:
ich benutze 'I...'

3:
je nach situtation mit oder auch ohne klammern. also vom beispiel genommen würde ich es so machen:

  while(true)
  {
    if(true) doSomething();
  }
  if(true)
    doMore(); 

4:

Was spricht denn hier für diese Vorgehensweise?

Folgendes:

mystring.Trim().Substring(...).Replace(...).ToUpper();

5:
siehe punkt 3

6:

public static bool TryXXX (...)
{
if(...) return false;
if(...) return false;
if(...) return false;
if(...) return false;
bool result;
// mach was
return result;
}

7:
ich benutze kein this und nur dann base, wenn es notwenig ist (also in override)

8:
ich benutze keine horizontale ausrichtung.

9:
je anch situation. bei strings z.b. ist es offensichtlich und einfach. bei komplexeren aufrufen teile cih diese auf, da sie sich so auch leichter debuggen lassen.

10:
nur wenn es sinn macht. ich werde mir keine methodennamen ausdenken, nur um eine prüfung machen zu können und ich werde mir auch keine methode mit 10 parametern anlegen, nur um zu prüfen, ob sie in der summe eine spezifische bedingung erfüllen.

muss ich diese bedingung allerdings mehrere male prüfen (in verschiedenen methoden), dann mache ich mir so eine methode.

1. Bei uns hat sich das folgendermaßen durchgesetzt und das passt mir auch ganz gut: Es gibt Find- und Get-Methoden, eine Get-Methode darf nie 'null' zurückgeben sondern knallt mit einer Exception wenn was nicht geklappt hat. Find-Methoden dürfen 'null' zurückgeben.

2. Finde ich komisch und belasse es lieber bei meinem 'I'.

3. Find ich praktisch und auch nicht wirklich unübersichtlich, ich denke das es oft egal ist wie das gemacht wird.

4. Meiner Meinung nach ist es übersichtlicher wenn das manipulierte Objekt auch zurückgegeben wird, vermindert sicher auch die Fehlerhäufigkeit bei anfängern da die meistens schon die string-Methoden gewöhnt sind und sich eine analoge Deklaration anbietet.

5. Ich setze die Klammern lieber untereinander, alles andere finde ich leicht unübersichtlich (evtl. auch nur ungewohnt).

6. Egal.

7. Diese Keywords verwende ich eigentlich nur wenn ich sie wirklich brauche (z.B. zum aufrufen des Basiskonstruktors)

8. Finde den ausgerichteten Code schöner und übersichtlicher, das einrücken dauert auch nicht die Welt.

9. Mache ich meistens über zwischenvariablen außer ich kann mir sicher sein das die Methode nicht null zurückgibt.

10. Wenn ich solchen Code öfter brauche lager ich es meistens in eine Read-Only-Property aus.

Punkt 1: wenn du noch zwischen null und leer unterscheidest, dann trägt das Ergebnis deiner Methode eine Meta-Information (nämlich darüber, ob bla nun true oder false war - was auch immer). Das ist unerwünscht, da dem anwendenden Programmierer diese Metainformation nicht mitgeteilt werden kann (nicht durch den Code).

Punkt 2: Interfaces IMMER mit I. Die Begründung des Autors ist schon sehr waghalsig, finde ich.

Punkt 3: Coding Convention bei uns: es wird immer geklammert. Ist auch sinnvoll, finde ich - bei c&p-Aktionen kann weniger daneben gehen, der Code kann erweitert werden, ohne dass man Klammern vergisst, so etwas hier passiert vermutlich weniger oft:

using(var irgendwas = blubb.ExecuteMethod()) irgendwas.blimm();
{
  /* codeblock - falscher Scope. */
}

Punkt 4: Cascading. Das Beispiel hat Jack schon erwähnt.

Punkt 5: normalerweise Java: öffnende Klammer in derselben Zeile, C#: in der nächsten Zeile.

Punkt 6: auf "single exit" zu pochen kann auch in sehr seltsamem Code gipfeln. Pragmatisch rangehen - die einfachste Lösung ist die beste.

Punkt 7: Klassenvariablen mit Klasse, Membervariablen ohne "this". Groß- und Kleinschreibungskonventionen reichen.

Punkt 8: Kommt noch aus der C-Programmierung. Nicht notwendig.

Punkt 9: Der Autor sollte sich mal klar werden, was er will: entweder Cascading und Punkt 4, oder Punkt 9. Beides gleichzeitig beisst sich ein klein wenig, finde ich.

Punkt 10: mit aussagekräftigen Namen, sicherlich. Macht den Code lesbarer.

Gruß,

LaTino

Hallo zusammen

Nur mal um Missverständnisse auszuräumen.
Das Buch ist für Java geschrieben, dort ist "I" keine Konvention, in .NET ist ein Präfix "I" Konvention von Microsoft und an allen Stellen empfohlen!

Zudem ist auch die Klammerungs-Konvention in Java und C# unterschiedlich, siehe unten.

1. Rückgabe von Object, das nie null ist, sondern "leere" Instanz

Das schliesse ich mit Jack und LaTino an.

2. Interfacenamen beginnen in einer Factory nicht mit dem "herkömmlichen" I, da es, so der Autor, der Benutzer nicht wissen muss ob es sich um ein Interface handelt, da dies ablenken könnte. Nun, ich benutze im Namen von Interfaces (ob Factory oder nicht) immer mit vorangestelltem I.

Siehe Anfang meines Postes.

3. Es werden bei einzeiligen Anweisungen, z.B. nach einer Schleife oder einer Bedingung keine Klammern gesetzt

Ich setze überall Klammern.
Begründung: Bessere Lesbarkeit und vielfach kommt später noch eine Anweisung dazu, dann steht die Klammer schon.

4. Wird ein Objekt übergeben (by Reference) und dieses manipuliert, dann soll dieses explizit zurückgegeben werden.

Beispielcode:

  
public StringBuffer ManipBuffer(StringBuffer inBuffer) { ...  
  

Was spricht denn hier für diese Vorgehensweise? Ich bin hier folgendes gewohnt:

So kannst du ein "Fluent Interface" bauen.
Ob das nötig ist, soll man von Fall zu Fall selber entscheiden.
Generell würde ich das überhaupt nicht vorschlagen. Denn wenn man die Rückgabe nicht benötigt, ist das Mist.

5. Ein heikles Thema, Klammerung. Ich habe anfänglich die Klammerung auf gleicher Spaltenebene (also untereinander) gesetzt (und lieben gelernt). Dann kam ich auf ein Projekt, wo die Klammerung so verwendet wurde, dass die öffnende Klammer in der gleichen Zeile gesetzt wird. Ich habe das dann so verinnertlicht, dass ich bei dem übernächsten Projekt mir Klammern untereinander nicht mehr aneignen wollte - das bläht den Code nur auf und entgegen vielen Behauptungen ist der Code nicht übersichtlicher. Wie macht ihr die Klammerung und warum ... man kann ja Gott sei Dank mit eigenen Formattern die IDE so einstellen, dass die Klammern nach Wunsch gesetzt werden.

Siehe am Anfang meines Posts.
In Java ist die Konvention "Klammer in der gleichen Zeile wie das Statement.
In C# ist die Konvention "Klammer in der nächsten Zeile".

Man sollte sich an die offizielle Konvention halten.
(Ich habe bis vor kurzem auch ersteres verwendet 😉)

6. Der Autor sagt, dass nicht immer ein einziges Return-Statement Sinn macht, z.B. bei kleinen überschaubaren Methoden,

Von Fall zu Fall engscheiden.
Generell so wenige return-Statements wie möglich.

Sooo, das reicht 😉

Gruss Peter

Hallo mosspower,

zu 1) Was genau zurück gegeben wird muss Definiert werden, d.h. beides kann richtig sein. Im zweifelsfall würde ich jedoch immer null vermeiden, da man so lästige NullReferenzExceptions vermeidet. (Besonders wenn Listen zurück gegeben werden!!)

zu 2) Die Begründung des Autors ist nachvollziehbar, man schreibt von ein Enum ja auch kein E . ABER bei interfaces gibt es normalerweise ja auch immer eine Implementation dazu, also müsste man laut des Autors nicht "IComparer / Comparer" sondern "Comparer / ComparerImplementation" schreiben. Also da sterb ich doch lieber den ersten Tod!

zu 3) Immer Klammern, das hat schon mein Lehrer in der 9. Realschulklasse gewuss! Keine Ahnung was für einen verdrehten Geist man braucht um ohne Klammern besser lesen zu können, Mr. 0815 kann auf jedenfall mit Klammern besser leben als ohne.

zu 4) Man kann mit so einer Schreibweise kürzeren Code generieren, da man den Parameter nicht in einer Variable halten muss und direkt weiter benutzen kann. Aber ich würde das nicht erwarten oder als da Non+Ultra bezeichnen. Man kann das so machen, ist aber nicht immer und nicht für jeden vorteilhaft.

zu 5) Ich klammere immer in einer neuen Zeile und öffnende und schließende Klammer untereinander. Ich kann das besser lesen - viel besser.

zu 6) Normalerweise kommt man immer mit einem return zurecht und das ist auch besser wart bar, da solche "kleinen Methoden" die Angewohnheit haben sehr schnell sehr groß zu werden!

zu 7) Ich verwende auch immer this, da man so Namenskonflikte vermeidet und sofort sieht, wenn man etwas außerhalb des aktuellen Blocks ändert (es ist ja ein himmelweiter unterschied, ob die Änderun am Ende der Methode einfach weg ist oder bestehen bleibt).
Das mit den Klassennamen finde ich auch gut, auch wenn ich es selbst noch nicht hin bekomme, das immer zu machen.

zu 8) Da man auf verschiedenen Monitoren und bei verschiedenen IDE-Einstellungen unterschiedlich viele Zeichen auf dem Monitor neben einander darstellen kann, ist das ohne hin für den Eimer, da das ganze auf dem nächsten System schon wieder nicht richtig dargestellt werden kann.

zu 9) Ich finde das extrem schrecklich. So entstehen nur 1000 Variablen, deren nutzen man nicht kennt und durch die man sich durch quwählen muss und zu allem überfluss wird die Methode auch noch länger und schwieriger zu lesen.
Man kann es natürlich mit den Einzeilern übertreiben, hier muss man ein gesundes Mittelmass finden. (Ich z.B. mache lieber "Zwischenvariablen" wenn der Aufruf länger als 40 (max. 80) Zeichen wird. )

zu 10) Arg! Wirklich nur wenn die Bedingung mehr als ein mal gebraucht wird. Ansonsten hab ich nur eine zusätzliche überflüssige Methoden, die A) irgend wo in die Klassendefinition muss aber nirgends hin gehört B) die Klassendefinition versaut und C) beim Lesen des Codes aufgeschlüsselt ("Gehe zu Definition") werden muss.

So und jetzt lese ich was der rest der Welt zu diesen Themen zu sagen hat. 🙂

Gruß
Juy Juka

1.: Ich halte es so wie Jack, etc.
2.: IMMER n I davor; warum ist ja mittlerweile oft genug gesagt worden
3.: Da ich faul und der Meinung bin, dass es die Lesbarkeit nicht vermindert nutz ich das manchmal
4.: das kommt wirklich auf die konkrete Situation drauf an, pauschal kann man das net sagen find ich
5.: Öffnende Klammer immer in der neuen Zeile, ist für mich deutlich lesbarer als die andere Variante, bei der ich immer n bissel durcheinander gerate ^^
6.: Auch da kann allgemein nicht viel zu sagen, außer, weniger returns( = besser, aber manchmal braucht man doch mehr als eines
7.: siehe Jack
8.: mach ich nicht, find ich auch net übersichtlicher
9.: auch hier kommt es auf die Situation drauf an, gerade bei irgendwelchen String-Methoden mach ichs meistens in einer Zeile; man sieht dann einfach auch teilweise schneller, was genau gemacht wird.
10.: Methoden genau dann, wenn ihre Funktionalität mehrfach gebraucht wird, dafür sind Methoden da.

Hallo mosspower,

1. Hier würde ich die Vorgehensweite des Autors unterstützen. Wenn ich an die Rückgabe von Listen, Arrays denke, relativiert sich m.E. zwar die Gefahr von NullReferenceExceptions schon allein dadurch, dass die Rückgabegröße des Arrays sowieso geprüft werden sollte - aber ob ich nun auf null oder Array.Count > 0 prüfe, ist eigentlich egal. Vorteil allerdings: wenn ich diesen Check vergesse (wie oft passier das mal im Eifer des Gefechts), laufe ich nicht in die angesprochenen NullReferenceExceptions rein.

2. Stimme ich den anderen voll und ganz zu. Das "I" gehört davor.

3. Schwierige Sache. Zum einen sind das sicher auch persönliche Vorlieben oder Vorgaben der Firma. Ich denke aber, Klammern muss sein. Schon allein, weil es ja doch recht oft passiert das man nachher eben noch die Schleife/... erweitert und dann ist die Gefahr von Leichtsinnfehlern weniger.

4. Kommt auf die Situation an. Generell aber will ich nicht immer einen Rückgabewert bekommen, wenn ich by reference arbeite.

5. Schwieriges Thema ja 😁 Jeder so wie er's mag oder wie's die Firma sagt (Code Conventions). Ich persönlich setze die Klammern in eine neue Zeile. Macht zwar mehr Codezeilen, ist aber ein wenig übersichtlicher. Meine Meinung.

6. Ich mag persönlichen den "Single Exit" lieber als viele kleine return's. Aber wenn's nicht anders geht, dann halt so.

7. Ich benutze immer this bzw. base. Macht den Code lesbarer finde ich. Was Klassenvariablen betrifft, benutzen wir in der Firma z.B. immer ein vorangestelltes "_" vor dem Variablennamen. So kann man diese auch gut unterscheiden.

8. Ausrichtungen bei Variablen gefallen mir nicht, aus den genannten Gründen. Bei Enumerationen mach ich sie aber recht gerne (wenn nicht XML-Kommentare vorgeschrieben sind).

9. Bevorzuge die kürzere Variante, wie Xand0r sagt vor allem bei String-Operationen. Bei anderen Operationen steige ich manchmal auf die längere Version um, damit das Debugging "einfacher" funktioniert.

10. Siehe Xand0r, JuyJuka etc.

Gruß Michbeck1983

Hallo mosspower,

ich schreibe nur zu den Punkten, wo ich ich meine neues oder abweichendes beitragen zu können:

Empfehlung 1 wäre nur sinnvoll, wenn man in einer idealen Welt lebt und wirklich nie und nirgends null zurückgegeben wird. Und wenn es eine einheitliche Vorgehensweise gegeben würde, um zu prüfen, ob ein leeren Objekt zurückgegeben wurde. Beide Kriterien erfüllt das .NET Framework nicht. Also ist keine Einheitlichkeit möglich, wenn man nicht komplett auf das .NET Framework verzichten will. Also ist die Empfehlung nicht sinnvoll. [EDIT]Eine Ausnahme bildet die Rückgabe von Collections, wo es sinnvoll seine kann, der Empfehlung zu folgen.[/EDIT]

Empfehlung 3 kann ich überhaupt nicht teilen. Es sollten immer überall Klammern gesetzt werden (einzige Ausnahme else if und da ist eine Ausnahme nur deshalb nötig, weil es in C# kein elif/elsif/elseif gibt). Vielleicht ganz interessant: Ich hatte ganz zu Anfang meines Studiums an einem Test teilgenommen. Dabei ging es darum zu erkennen, was einige kurze Programme tun, deren Code auf dem Bildschirm angezeigt wurde. Allerdings konnte man zu einer Zeit immer nur eine Zeile des Codes sehen. Ging man mit dem Cursor nach unten, sah man die nächste Zeile, aber die davor verschwand. Beim Aufwärtslaufen entsprechend. Das größte Problem für mich waren genau die Stellen, wo nach einem if oder while keine Klammern verwendet wurden. Dort bin ich regelmäßig mit den Hierarchieebenen durcheinander gekommen. Für mich spätestens ab da Grund genug, immer Klammern zu setzen.

Empfehlung 6 kann ich überhaupt nicht teilen. Überall, wo return möglich ist, sollte man es benutzen. Jedes return schneidet einen Code-Pfad ab, denn man beim Lesen des Codes andernfalls unnötigerweise im Kopf behalten müsste. Und spätestens mit finally ist das letzte Argument weggefallen, das für ein einziges return am Ende sprach, nämlich eine zentrale Stelle für abschließenden und Aufräumarbeiten zu haben.

herbivore

Hallo,

ad 1: Kommt darauf an. Ich gebe bei Methoden, die ein einzelnes Objekt zurückgeben, null zurück, bei Methoden, die eine Liste zurückgeben, eine leere Liste. Sinn: Auf null prüfen muss man sowieso, das ist IMHO guter Stil, es spart Speicher, aber eine Liste steckt man ziemlich oft direkt in foreach rein - dort würde es mit null dann knallen.

Ad 2: Interfaces ohne I widersprechen den Microsoft-Richtlinien für .NET. Mag woanders sinnvoll sein, aber IMHO sind die MS-Richtlinien speziell im Hinblick auf .NET als höhergewichtig anzusehen.

Ad 3: Halte ich für eine sehr schlechte Idee. Code sollte so geschrieben sein, dass er gut lesbar ist, und Klammern unterstützen die Lesbarkeit und vermeiden bei Editoren ohne Syntax Highlighting potenzielle Fehler.

Ad 4: Du übergibst in Deinem Beispiel das Objekt nicht by Reference, sondern by Value. Um es by Reference zu übergeben, musst Du das ref-Schlüsselwort verwenden. Ansonsten: Fluent Interfaces, wie schon erwähnt.

Ad 5: Wie 2, Standard in .NET ist nun mal, die öffnende Klammer in eine neue Zeile zu machen.

Ad 6: Multiple return-Statements sind IMHO durchaus legitim. Es ist immer abzuwägen, was übersichtlicher ist - ein einzelner Point of Return, oder mehrere Returns.

Ad 7: Ich schreibe this immer dazu, weil es bei Properties ansonsten zu Uneindeutigkeiten kommen kann (ist Foo in Foo.Bar() jetzt eine Property oder eine Klasse mit einer statischen Methode Bar?).

Ad 8: Halte ich für Quatsch, weil das von der Schriftart abhängt, die man eingestellt hat. Wenn man mit einer Proportionalschrift wie zB Verdana programmiert, zerschießt es einem das schönste Layout. Ich verwende nur ein Leerzeichen, und nur am Zeilenanfang Tabs, wobei ein Tab 4 Leerstellen hat.

Ad 9: Die ausführliche Version ist besser zu debuggen, aber nicht unbedingt übersichtlicher. It depends 😉

Ad 10: Ja, sofern die Bedingung mehr als einmal geprüft wird (Stichwort Refactoring: Extract Method).

Viele Grüße,

Golo

ad 1: Kommt darauf an. Ich gebe bei Methoden, die ein einzelnes Objekt zurückgeben, null zurück, bei Methoden, die eine Liste zurückgeben, eine leere Liste. Sinn

Genauso handhab ich das auch. Eine leere Liste hat ja primär erstnmal nur eine Containerfunktion, und beherbergt andere Objekte. Wenn keine anderen da sind, ist sie leer. Aber ein foreach läuft prim darüber, auch wenn nichts weiter passiert.
Bei normalen Objekten geb ich aber null zurück, wenn es keinen Sinn macht, überhaupt etwas zurückzugeben. Wenn nicht alle Bedingungen erfüllt sind, warum also ein leeres Objekt zurückgeben: Denn auch ein leeres Objekt beinhalte Informationen, und es könnten Methoden aufgerufen werden, die gar nicht sinnvoll sind oder gar zu Fehlern führen. Dann lieber null zurückgeben, und wissen, dass die Klasse halt nicht instanziert wurde.

Hallo Golo Roden,

Ad 4: Du übergibst in Deinem Beispiel das Objekt nicht by Reference, sondern by Value. Um es by Reference zu übergeben, musst Du das ref-Schlüsselwort verwenden. Ansonsten: Fluent Interfaces, wie schon erwähnt.

in dem Beispiel wird der Variableninhalt in der Tat by Value übergeben. Da der Variableninhalt aber eine Referenz auf ein Objekt ist, wird das Objekt by Reference übergeben, so wie mosspower das gesagt hat. Wenn man noch ein ref dazuschreiben würde, würde die Variable per Reference übergeben, so dass der Varableninhalt (also die enthaltene Reference) innerhalb der Methode überschrieben werden kann. Das ist hier aber nicht gemeint.

herbivore

1. Ich bin der Meinung, man muss beim Programmieren ganz klar zwischen leer und nicht angegeben/nicht da/nichts unterscheiden. Bei Collections mag das vllt. noch vertretbar sein, dass man etwas leeres über- oder zurückgibt. Bei Datenklassen ist es absolut schwachsinnig leere Instanzen zu erstellen, oder macht es vllt. für euch einen Sinn eine leere Adresse oder leeren Benutzer zu übergeben?

Weiterhin muss man in vielen Anwendungsfällen dennoch prüfen, ob eine Instanz "leer" oder nicht angegeben wurde. Wird dabei ein neuer Zustand (leer) eingeführt, hat man meißt sogar 2 Vergleiche, da man sich selbst und anderen nicht über den Weg traut.

Weiterer Punkt ist, dass ich je nach Anwendungsfall den Speicher mit sinnlosen Objekten zumülle. Ein nothing muss hingegen nicht vom GC weggeräumt werden.

Edit:
Wo es abosolut Sinn macht, sind Strings. Es gibt Zeichenketten die leer sind und welche die nicht angegeben worden sind.

@herbivore: Stimmt schon, aber ich glaube, diese Feinheit ist oft nicht klar. Und dann wundert man sich halt leicht, warum zB ein foo = null keine Auswirkung "weiter oben" hat, obwohl das Objekt doch vermeintlich by Reference übergeben wurde.

Dazu benötigt man dann in jedem Fall das ref-Schlüsselwort.

@Peter Bucher: Das dürfte die Checkbox gewesen sein

4. Wird ein Objekt übergeben (by Reference) und dieses manipuliert, dann soll dieses explizit zurückgegeben werden.

Beispielcode:

  
public StringBuffer ManipBuffer(StringBuffer inBuffer) { ...  
  

Was spricht denn hier für diese Vorgehensweise? Ich bin hier folgendes gewohnt:

  
public void ManipBuffer(StringBuffer inBuffer) { ...  
  

Wenn hier nur die Manipulation des Objektes an sich gemeint ist (und nicht der Variablen -> ref), dann macht die Rückgabe des übergebenen Parameters eigentlich nur Sinn, wenn der Parameter eine Kopie ist (z.B. bei Remoting) und man die Zuständsänderung explizit zurückgeben müsste, da auf dem Client noch das Originalobjekt besteht.

1. Rückgabe von Object, das nie null ist, sondern "leere" Instanz

Halte ich eigentlich immer ein, damit brauche ich in der Funktion, die die Funktion aufruft, kein Exception-Handling und keine Prüfung auf null mehr vorsehen. Finde ich übersichtlicher und es macht weniger Aufwand.

2. Interfacenamen beginnen in einer Factory nicht mit dem "herkömmlichen" I, da es, so der Autor, der Benutzer nicht wissen muss ob es sich um ein Interface handelt, da dies ablenken könnte.

Ich benutze auch ein I. Es kann dem Benutzer sehrwohl dienlich sein zu wissen, was ein Interface ist und was nicht, denn von einem Interface kann er keine Instanz erzeugen sondern muss es erst implementieren. Der Autor geht wohl davon aus, dass das Interface nur als Rückgabewert einer Funktion dient. Ich weiß auch garnicht was an dem I ablenkend sein könnte...

3. Es werden bei einzeiligen Anweisungen, z.B. nach einer Schleife oder einer Bedingung keine Klammern gesetzt

Das sieht man sehr häufig, ich habe persönlich die Erfahrung gemacht dass mit Klammern einfach übersichtlicher ist und deshalb mache ich auch immer welche. Es kommt so häufig vor dass man doch mal noch eine Zeile hinzufügt und schwupps hat man die Klammerung vergessen und es passiert beim Ausführen was ganz anderes als gewollt, oder man setzt die Klammern nachträglich an die falsche Stelle.

4. Wird ein Objekt übergeben (by Reference) und dieses manipuliert, dann soll dieses explizit zurückgegeben werden.

An dieser Stelle würde ich dann im Zweifelsfall den ref-Operator verwenden, da ich als Rückgabewert oft einen ReturnCode verwende der mir sagt ob überhaupt und was schiefgegangen ist. Aber darüber lässt sich streiten, wenn man in seinem Code nicht richtig durchblickt 😉 oder wenn man den Code vielleicht weitergibt dann ist der Tipp aus dem Buch nicht ganz verkehrt. Aber wenn man die Funktion richtig kommentiert...

5. Ein heikles Thema, Klammerung.

Ich mache die Klammern untereinander. Dann sehe ich gleich wo sie aufgeht und wo sie zugeht. Mit Klammerung hinter dem Statement komme ich garnicht klar, wenn ich so Beispielcode sehe kopiere ich ihn gleich in VS und lasse dort formatieren.

6. Der Autor sagt, dass nicht immer ein einziges Return-Statement Sinn macht, z.B. bei kleinen überschaubaren Methoden.

Ich habe mich anfangs jahrelang damit geplagt auch immer nur ein Return zu verwenden, weil es in vielen Ratgebern als "sauber und übersichtlich und am sichersten" empfohlen wurde. Diese Erfahrung kann ich nicht teilen, hat bei mir resultiert in tiefe Verschachtelung, ein einziger Klammerhaufen. Hab dann damit angefangen mit in einer Funktion "Status"-Variablen anzulegen um die Klammern zu vermeiden, sozusagen if (goOn) tu was. Aber das fand ich dann nicht wirklich sauber... jetzt gönne ich mehrere returns und finde den Code viel übersichtlicher.
Und Probleme hatte ich bisher damit noch nicht.

7. Zugriffe auf Variablen.

Ich schreibe nur base davor, weil es ja da quasi nicht anders geht. ob this oder nicht this, es geht ja beides, und nachher vergess ich eins und bin dann verwirrt und interpretiere den Wert falsch...
Was hier vielleicht auch noch dazupasst ist die Frage, schreibe ich den kompletten Namespace davor (trotz using ...), oder nur den Typ? Bei Fremd-Beispielcode wär mir zweites oft lieber, aber ich selbst mache es z.B. auch nie weil sonst die Zeilen elendig lang sind...

8.) Horizontale Ausrichtung

Find ich umständlich, es kommen ja immer mal wieder neue hinzu, und dann wird die Einrückung ggf. immer größer, oder es fallen welche weg und man hat unnötig viel Einrückung. Horizontale Ausrichtungen versuche ich wenn dann höchstens bei Enum-Werten oder zusammengehörigen Konstantenwerten zu erreichen, da finde ich es dann schon übersichtlich.

9. Mehrfachaufrufe in einer Zeile

wenn ich weiß dass es nicht krachen kann, dann würde ich die kürzere Variante verwenden. Die Variante vom Buch legt mir zu viele unnütze Variablen an. Macht den Code nur unnötig länger.

10. Komplexe Bedingungen sollten in eine Methode ausgelagert werden.

Wenn mehrfach benutzt ja, anderenfalls mache ich es eher selten, aber es stimmt durchaus, dass man bei korrekter Benamsung so einer Funktion gleich wüsste was sie macht, was bei einer komplexen verschachtelten Bedingung nicht unbedingt sofort der Fall ist. Also kann man machen, ist wahrscheinlich auch sinnvoll sobald eine bestimmte Komplexität erreicht ist, aber die Faulheit wird viele (auch mich oft) davon abhalten

gruß
sth_Weird

10. Macht nicht nur Sinn, bei Mehrfachverwendeung sondern auch bei komplexeren Bedingungen. Die Methode würde dann so heißen, wie das, was abgefragt wrden soll. So würde aus.

if(collection != null && collection.Count > 0){

}

if(CollectionIsNotNullOrEmpty(collection)){

}

werden.

Also die Bedingung ist jetzt nicht so komplex aber ihr wisst schon was ich meine.
Ich finde es erhöt die lesbarkeit. Nach mehreren Monaten kann es schnell passieren, dass man nicht mehr weis was die Bedingung genau abprüft.

mfg
Zebes

Ich finde es erhöt die lesbarkeit. Nach mehreren Monaten kann es schnell passieren, dass man nicht mehr weis was die Bedingung genau abprüft.

problematisch wirs nur bei solchen bedingungen:

if(MyItemHasNoInvalidValuesAndIsAboveThousendAndUnderFifeThousendAndIsNotPrime(myitem))

oder noch besser:

if(IsNotNullHasNoInvalidArgumentsAndIsBetweenThirdValidScope(myvalue))

Dann würde der boolsche Ausdruck aber auch der absolute Overkill sein. Ich würde die einzelnen Bedingungen in jeweils eine Methode packen. In der If-Abfrage würde bei sowas vermutlich nur noch

if(ObjectIsValid(objekt)){

}

stehen.

Und ObjectIsValid würde in etwa so aussehen:

bool ObjectIsValid(object objekt){
    return ObjectHasNoInvalidValues(objekt) && 
              ObjectIsAboveThousand(objekt) && 
              ObjectIsNotPrime(objekt);
}

mfg
Zebes

@Zebes

genau, und jetzt fangen die probleme damit erst an 😃

wenn du noch die prüfung aus dem konkreten beispiel verfollständigst und auf unter 5000 prüfst (also 1 methode mehr) und du sämtliche prüfungen nur einmal benötigst, dann hast du in diesem fall 6 private methoden, die du nur an einer stelle benötigst und in der aufrufenden methode siehst du nur "ObjectIsValid" was absolut nichtssagend ist, da du ja einen bestimmten validen zustand abrufst und nciht einen allgemeinen.

daher bleibe ich dabei, das wenn sich kein vernünftiger name finden lässt, der auch ohne widescreen lesbar ist, ich lieber die prüfung so lasse (vorrausgesetzt die prüfung ist nur einmal nötig) und lieber einen sprechenden kommentar formuliere.

Also würdest du lieber einen boolschen Ausdruck da hin setzen? Finde ich bei weitem unleserlicher. Wenn die Bedingungen zu Komplex werden, würde ich mir eine andere Validierungsstrtegie überlegen. Mal schauen wie das mit einem boolschen Ausdruck aussehen würde.

if(myItem.Find(value => !value.IsValid) == null && 
   myItem > 1000  && 
   myItem < 5000 &&
   !myItem.IsPrime){

}

Naja wem es gefällt. Aus Erfahrung weis ich, dass die Bedingungen nicht ganz so simpel zu verstehen bleiben wie hier. Von daher bevorzuge ich an den Stellen doch eher die Variante mit einer Methode.

prinzipiell ja, mit folgendem kommentar:

// if all values are valid, between 1000 and 5000 and not a prime
if(myItem.Find(value => !value.IsValid) == null &&
   myItem > 1000  &&
   myItem < 5000 &&
   !myItem.IsPrime){

}

Also im besten Falle würde aus den Methoden namen noch hervorgehen, warum das Objekt genau so aussehen muss.

Natürlich würde ich keine Methode draus machen wenn die Abfrage zu simpel ist, es kommt halt immer aud den Fall an. Ich extrahiere spätestens dann eine Methode, wenn ich erneut auf den Code schaue und erst mal überlegen muss wofür die Bedingung noch gut war.

Es heist in einigen Ratgebern, dass wenn man den Code nur mit Hilfe eines Kommentares versteht, es sich um sogenannten "smelling code" handelt. Dann sollte man sich doch Gedanken um ein Refactoring machen. Zumindest würde ich das tun.

Es heist in einigen Ratgebern, dass wenn man den Code nur mit Hilfe eines Kommentares versteht, es sich um sogenannten "smelling code" handelt.

dann müsste ich jede 2. zeile refactoren. zudem ist diese pauschalaussage äußerst schwammig. ich kann auch assembler verstehen, das heißt aber nciht, das ein kommetar deswegen nicht notwendig ist. kommentare sollen einem helfen, den grund zu erfahren und das kann man nur schlecht durch einen methodennamen ausdrücken. ein methodenname soll seine funktion ausdrücken und nciht den grund. ist seine funktion jedoch zu komplex, muss man auf kommentare zurückgreifen oder die methode aufsplitten. da aber eine prüfbedingung sehr komplex, in der funktion aber meist sehr simpel ist, ist die wahl einer methode im falle einer einzigen verwendung meiner ansicht nach nicht sinnvoll.