Re: [ccd] Exception Handling statt Return Value Auswertung

[Christof Konstantinopoulos]

Hallo Marco,

wenn ich deine Frage richtig deute, soll die aufgerufene Methode etwas
mit einer Datei tun und falls die daf�r notwendige Datei nicht vorhanden
ist, dann gibt sie einen Fehlercode als R�ckgabewert zur�ck.

Als Beispiel stelle ich mir eine Methode "appendTextToFile(Filename,
TextToAppend)" vor. Deren Aufgabe ist einfach das Anh�ngen der Daten.
Wenn es nicht Klappt, dann kommt ein R�ckgabewert als Fehlerkennung.

Und das ist meiner Meinung nach falsch. Es m�sste eine Exception kommen.
Denn eigentlich sollte die Pr�fung auf das Vorhandensein der Datei
vor(!) dem Arufruf der Methode "AppendTextToFile" durchgef�hrt werden.
Und wenn die Datei dann nicht da ist, dann d�rfte die Methode gar nicht
aufgerufen werden.

Die Pr�fung FileExist ist eine eigene Methode. Deren Aufgabe ist es,
anhand eines R�ckgabewertes, den Programmablauf zu steuern. Und wenn die
Pr�fung ergibt "Datei nicht da", dann darf "appendTextToFile" nicht
aufgerufen werden. Dann gibt es auch keine Exception.
Wenn aber FileExist ergibt, dass die Datei da ist, dann muss ein Fehler
bei "appendTextToFile" als Exception geworfen werden.

Viele Gr��e,
Christof


Am 28.01.2013 09:55, schrieb Marco Feltmann:
> Moin,
>
> ich stecke in einem Dilemma bez�glich der Do's und Don'ts im Standardwerk.
> Dort hei�t es, das Pr�fen der R�ckgabewerte sei zu vermeiden. Die
> damit einhergehenden IF-Pr�fungen seien ein Zeichen f�r schlechten
> Code, da die Methode zumindest eine Sache mehr tut als sie soll: etwas
> �berpr�fen.
>
> Nun habe ich die gro�e Ehre in der Hauptsache in einer
> Programmiersprache zu entwickeln, in der Exceptions getreu ihres
> Namens Ausnahmeerscheinungen bilden und gem�� Sprachdesign nahezu
> alles �ber R�ckgabewerte oder Fehlerobjektreferenzen gel�st wird.
> Pers�nlich finde ich diese L�sung prima, denn FileNotFound ist eine
> M�glichkeit und kein schwerer Ausnahmefehler. Eine simple Pr�fung auf
> die Existenz der Datei umgeht diese Exception bereits.
>
> Doch wie passen die CCD-Prinzipien auf diesen Sonderfall?
>
> Fragende Gr��e
> Marco
> --
> Sie haben diese Nachricht erhalten, weil Sie der Google Groups-Gruppe
> Clean Code Developer beigetreten sind.
> Wenn Sie Nachrichten in dieser Gruppe posten m�chten, senden Sie eine
> E-Mail an clean-code...@googlegroups.com.
> Wenn Sie aus dieser Gruppe austreten m�chten, senden Sie eine E-Mail
> an clean-code-devel...@googlegroups.com.
> Gruppe besuchen: http://groups.google.com/group/clean-code-developer?hl=de
> Weitere Optionen: https://groups.google.com/groups/opt_out
>
>

[Ralf Westphal]

Welche CCD Prinzipien könnten denn passen?

SRP zum Beispiel. Oder PoLA.

Christof hat da schon was angedeutet.

Ist es überraschend, wenn eine Datei nicht existiert? Ja. Dann sollte solche Ausnahme mit einer Ausnahme angezeigt werden. Das ist dann wiederum nicht überraschend ;-) Also least astonishment.

Ist die Nichtexistenz hingegen quasi ein Normalfall, dann keine Ausnahme. Dann ein anderer Weg der Anzeige, zum Beispiel:

if (!TryAppendToFile(filename, text)) { ... }

Jetzt die Frage, wer feststellt, ob die Datei vorhanden ist. Bleiben wir beim Beispiel "Anhängen an Datei".

Das ist eine single responsibility. Nach außen.

Aber innen? Da gibt es zwei responsibilities: Prüfung der Existenz der Datei und das tatsächliche Anhängen.

bool TryAppendToFile(string filename, string text) {

  If (File.Exists(filename)) {

    AppendToFile(filename, text);

    return true;

  }

  else

    return false;

}

Und die sollten klar getrennt sein. Wie oben sähe es normal aus. Aber da ist für mich noch das Integration Operation Separation Principle verletzt :-) (nein, das gehört nicht zu CCD)

Die Methode tut nämlich zweierlei: sie enthält Logik und sie integriert andere Methoden. Darüber könnte man nachdenken, ob noch aufgelöst werden könnte. Auf der Granularitätsebene würd ich das nicht machen in der Praxis. Aber es könnte theoretisch so aussehen:

bool TryAppendToFile(string filename, string text) {

  var result = false;

  Check_file_existence(filename, () => {

      AppendToFile(filename, text);

      result = true;

  });

  return result;

}

Jetzt integriert die Methode nur noch. Sie enthält keine Logik mehr, also keine Kontrollstrukturen und keine Ausdrücke. Sie fügt nur noch Teilfunktionalität (Operationen) zu einem neuen Größeren zusammen (Integration).

Am Montag, 28. Januar 2013 09:55:15 UTC+1 schrieb Marco Feltmann:

Moin,

ich stecke in einem Dilemma bezüglich der Do's und Don'ts im Standardwerk.

Dort heißt es, das Prüfen der Rückgabewerte sei zu vermeiden. Die damit einhergehenden IF-Prüfungen seien ein Zeichen für schlechten Code, da die Methode zumindest eine Sache mehr tut als sie soll: etwas überprüfen.

Nun habe ich die große Ehre in der Hauptsache in einer Programmiersprache zu entwickeln, in der Exceptions getreu ihres Namens Ausnahmeerscheinungen bilden und gemäß Sprachdesign nahezu alles über Rückgabewerte oder Fehlerobjektreferenzen gelöst wird.

Persönlich finde ich diese Lösung prima, denn FileNotFound ist eine Möglichkeit und kein schwerer Ausnahmefehler. Eine simple Prüfung auf die Existenz der Datei umgeht diese Exception bereits.

Doch wie passen die CCD-Prinzipien auf diesen Sonderfall?

Fragende Grüße

Marco

[Marco Feltmann]

Ich danke euch beiden für eure Antworten.

Zunächst aber zu einem sehr wichtigen Punkt:

Am Montag, 28. Januar 2013 21:41:15 UTC+1 schrieb Christof Konstantinopoulos:

Und das ist meiner Meinung nach falsch. Es müsste eine Exception kommen.

Leider spielt die persönliche Meinung hier keine Rolle. Es handelt sich ja weder um Java noch um C#.

Und die Designer der Sprache dachten sich halt:

"Instead of exceptions, error objects (NSError) and the Cocoa error-delivery mechanism are the recommended way to communicate expected errors in Cocoa applications. For further information, see Error Handling Programming Guide."

Im Klartext: Probleme, die erwartet werden können, sollen über ein Error Handling kommuniziert werden.

Dies ist ein integraler Bestandteil der Sprache und des Frameworks, ist definitiv in Stein gemeißelt und lässt sich nicht ändern.

Beispiel mit dem Schreiben von Binärdaten in eine Datei:

NSError * writingError = nil;

BOOL isWritingSuccessful = [myDataInstance writeToFile:@"~/myData.bin" options:NSDataWritingAtomic error:&writingError];

In diesem Fall tut die Standardimplementierung sicherlich Dinge: sollte die Datei ~/myData.bin nicht existieren, dann wird sie halt angelegt.

Des Weiteren wird auf Grund der gesetzten Options erst einmal $irgendwo die Datei geschrieben und nach Abschluss gen ~/myData.bin verschoben.

Der Rückgabewert der Methode zeigt an, ob der Vorgang erfolgreich war und die Existenz des writingError Objektes teilt mit, dass es Fehler gab und wie diese aussehen.

Error stellt die Instanzmethoden -code, -domain und -userInfo bereit, die sehr zuverlässig ausplaudern, was wo wie wann warum schief gegangen ist.

Doch was kann hier schief gehen?

• die Datei existiert bereits, kann aber nicht überschrieben werden, zum Beispiel auf Grund eines Rechteproblems

• die Datei existiert nicht, kann aber nicht erstellt werden, zum Beispiel auf Grund eines Rechteproblems

Dies sind im Sinne der Dokumentation 'expected errors', also Dinge, die zu erwarten sind.

Man muss immer davon ausgehen, dass der User versucht auf ein nur lesend eingebundenes Netzlaufwerk zu schreiben oder einen schreibgeschützten USB-Stick als Zielpfad angibt.

Das ist meines Dafürhaltens kein schwerer Ausnahmefehler. Hier muss kein Speicher aufgeräumt, das Programm in einen sicheren Zustand gepackt und alles an Daten gesichert werden.

Hier reicht es den User darauf hinzuweisen, dass die von ihm gewählte Datei nicht erstellt werden konnte, weil ein Rechteproblem vorliegt.

Nun hat natürlich der Nutzer keinen Einfluss darauf, wenn ich 'unter der Haube' Daten in seinem Home-Verzeichnis ablegen möchte - in dem Fall wäre eine Meldung bezüglich des Rechteproblems widersinnig.

Nur bin ich ja nicht gezwungen den aufgetretenen Fehler weiter zu reichen.

Ich sehe in diesem Anwendungsfall schlicht keinen Grund darin, es als 'Ausnahmefehler' zu deklarieren.

Anstatt dass ICH versuche alle Eventualitäten abzudecken um sicherzugehen, dass mein Schreibvorgang erfolgreich ist, macht das einfach der Schreibvorgang für mich.

Meiner Meinung nach ist das die Art und Weise, auf die sauberer Code nach außen hin funktionieren soll. Die Methode verspricht, in eine Datei zu schreiben.

Die Methode tut alles erdenkliche, um dieses Versprechen zu halten. Sollte sie dieses Versprechen nicht halten können, so entschuldigt sie sich mit einer Begründung bei mir.

Wie oft hatte ich einen kleinen Fehler in meinem Pfad und friemelte an der FileNotFound Exception Javas herum bis ich den Fehler fand - und wie schnell zeigte mir eine Fehlermeldung direkt auf, dass ich einen Tippfehler im Pfad hatte.

 

Bei dem Ansatz haben alle beteiligten Methoden genau eine Aufgabe:
FileExist : Test, ob eine Datei vorhanden ist
die aufrufende Methode : Verarbeitung der Antwort von FileExist
appendTextToFile : Anhängen von Text

Wie bereits ausgeführt habe ich auf dieses Verhalten keinerlei Einfluss, es ist so im Sprach- und Frameworkstandard definiert.

Meine Frage ist, wie ich diesen gegebenenfalls bekommenen Fehler nun auswerten kann.

Ich müsste ja zumindest den boolschen Rückgabewert und den Inhalt des Error-Objektes überprüfen.

Prüfe ich vorher Dateiexistenzen, Dateizugriffsrechte, Ordnerzugriffsrechte, relative Luftfeuchtigkeit und mittleren Hochwasserstand, täte ich Dinge doppelt, die die Methode des Framework bereits durchführt.

Ich hätte also doppelten Code, selbst wenn ich ihn nur ein mal geschrieben habe. Ich hätte doppelte und dreifache Sicherungen eingebaut und damit zumindest das DRY verletzt.

Wir erinnern uns: die Methode verspricht die Datei zu schreiben - komme was wolle - und teilt uns ein eventuelles Scheitern mit Erläuterungen mit.

Zurück zum Eingangsproblem fällt mir als Lösung nur eine abstrakte Hilfsklasse ein, die sich um die Auswertung kümmert.

NSError * writingError = nil;

BOOL isWritingSuccessful = [myDataInstance writeToFile:@"~/myData.bin" options:NSDataWritingAtomic error:&writingError];

[ErrorHandlerClass logError:writingError operationSucceeded:isWritingSuccessful];

// --- Hilfsklasse ErrorHandlerClass mit ihren Klassenmethoden --- //

+ (void)logError:(NSError*)caughtError operationSucceeded:(BOOL)isSuccessful

{

  // Operation was successful, no error logging needed.

  if(isSuccessful)

  {

    return;

  }

  // Error object empty, no error logging needed as well.

  if(caughtError == nil)

  {

    return;

  }

  [ErrorHandlerClass logInformationForError:caughtError];

}

+ (void)logInformationForError:(NSError*)theError

{

  NSLog(@"### Error occured ###");

  NSLog(@"[Domain %@] | [Code %@]", [theError domain], [theError code]);

  NSLog(@"- User Info -");

  NSLog(@"%@", [self stringForUserInfo:[theError userInfo]);

}

+ (NSString*)stringForUserInfo:(NSDictionary*)userInfo

{

  NSMutableString * resultString = [NSMutableString string];

  for(NSString * key in userInfo)

  {

    [resultString append:key];

    [resultString append:@": "];

    [resultString append:[userInfo objectForKey:key]];

    [resultString append:@"\n"];

  }  

  return resultString;

}

// --- End Of Class --- //

Nur: wäre das im Sinne von CCD?

[Marco Feltmann]

Am Montag, 28. Januar 2013 22:38:16 UTC+1 schrieb Ralf Westphal:

Welche CCD Prinzipien könnten denn passen?

SRP zum Beispiel. Oder PoLA.

Nun ja, 'single responsibility' heißt, dass diese Methode genau für diese eine Aktion verantwortlich ist - und wenn die Aktion 'appendToFile' heißt, dann soll sie verdammt noch mal an diese Datei anhängen.

Ich möchte mich schon darauf verlassen können, dass diese Methode das macht. Und zwar ohne dass ICH dafür Sorge tragen muss, dass SIE ihre versprochene Aktion auch durchführen kann.

Auch erstaunt es mich weniger, dass eine appendToFile Methode ohne Murren so gut sie kann an etwas an eine Datei anhängt als dass sie mich mit 17 möglichen Exceptions ärgert.

Beispielsweise wundere ich mich unter Java immer, warum sie ihre FileNotFoundException werfen anstatt sie einfach im Vorfelde auszuschließen.

 

Ist es überraschend, wenn eine Datei nicht existiert? Ja. Dann sollte solche Ausnahme mit einer Ausnahme angezeigt werden. Das ist dann wiederum nicht überraschend ;-) Also least astonishment.

Ist die Nichtexistenz hingegen quasi ein Normalfall, dann keine Ausnahme. Dann ein anderer Weg der Anzeige, zum Beispiel:

Nein, es ist nicht überraschend. Es ist der Normalfall eines frisch partitionierten Datenträgers, eines gerade angelegten Ordners oder eines jungfräulich gemounteten Laufwerks.

Einer appendToFile sollte es auch egal sein, ob die Datei schon existiert: wenn ich 'Marco was here' an eine Datei mit Inhalt '2013AD: ' anhänge, erwarte ich genauso ein Ergebnis wie an eine Datei mit Inhalt '' und in logischer Konsequenz auch an keine Datei. 

Ich erwarte, dass nach Aufruf die Datei namens filename an Position EOF-'Marco was here'.length() den Text 'Marco was here' hat.

Ich erwarte nicht das Fehlen einer Datei namens filename. Ich erwarte von der Methode appendToFile einen Erfolg.

Sollte das nicht gehen, dann ist das zunächst kein schwerer Ausnahmefehler. 

Die Datei existiert gar nicht? Leg sie an. 

Das Programm hat keine Rechte dafür? Dann habe ich mich wohl beim Pfad vertan. Ist zwar ein Fehler, aber sicherlich keine schwere Ausnahme.

if (!TryAppendToFile(filename, text)) { ... }

Diese Namensgebung finde ich ausgesprochen uneindeutig. Was liefert denn tryAppendToFile?

Gelesen ergibt dies: if not tr(ied)AppendToFile = wenn nicht versucht wurde, an Datei anzuhängen.

Dies ist aber faktisch falsch, denn es wurde ja versucht und das Ergebnis wird zurückgegeben.

if(!SuccessTryingAppendToFile(filename, text))...

Gelesen ergibt dies: if not success(ful)TryingAppendToFile = wenn es nicht erfolgreich war, an Datei anzuhängen.

Nur: wenn es nicht erfolgreich war, an die Datei anzuhängen, dann wurde es ja auch nicht gemacht.

if(!AppendToFile(filename,text))...

Gelesen ergibt dies: if not append(ed)ToFile = wenn nicht an Datei angehängt wurde.

Auch wenn jetzt nirgendwo hervor geht, dass diese Methode versucht anzuhängen, sagt der Name doch genau das aus, was diese Methode tut und was sie im Fehlerfall halt nicht tat.

 

Jetzt die Frage, wer feststellt, ob die Datei vorhanden ist. Bleiben wir beim Beispiel "Anhängen an Datei".

Das ist eine single responsibility. Nach außen.

Aber innen? Da gibt es zwei responsibilities: Prüfung der Existenz der Datei und das tatsächliche Anhängen.

bool TryAppendToFile(string filename, string text) {

  If (File.Exists(filename)) {

    AppendToFile(filename, text);

    return true;

  }

  else

    return false;

}

Exakt, doch wie lassen sich die Prüfungen und die Aktionen am Besten im Sinne von CCD trennen?

 

Und die sollten klar getrennt sein. Wie oben sähe es normal aus. Aber da ist für mich noch das Integration Operation Separation Principle verletzt :-) (nein, das gehört nicht zu CCD) 

Die Methode tut nämlich zweierlei: sie enthält Logik und sie integriert andere Methoden. Darüber könnte man nachdenken, ob noch aufgelöst werden könnte. Auf der Granularitätsebene würd ich das nicht machen in der Praxis. Aber es könnte theoretisch so aussehen:

bool TryAppendToFile(string filename, string text) {

  var result = false;

  Check_file_existence(filename, () => {

      AppendToFile(filename, text);

      result = true;

  });

  return result;

}

Danke für die Gedanken. Doch: einerseits finde ich diese Teilfunktionalität (Closures, richtig?) nicht unbedingt leserlich und anderseits wird die Logik vielmehr versteckt ohne sie komplett zu entsorgen.

Es macht den Code für mich sehr viel komplexer und weniger intuitiv.

Davon ab gäbe es die Möglichkeit dieser 'Inline-Methoden' für mich auch gar nicht.

Es wäre also eher ein

- (BOOL)appendString:(NSString*)textString toFile:(NSString*)filepath

{

  BOOL succeeded = NO;

  [MyHelperClass checkExistenceOfFileAtPath:filepath onSuccessPerformBlock:^(void){

    [self appendString:textString toFileContentAtPath:filepath];

    succeeded = YES;

  }];

  return succeeded;

}

Oder so ähnlich...

Nur schiebe ich hier die Kontrollstrukturen wieder eine Ebene weiter - wie sollte nun meine checkExistenceOfFileAtPath:onSuccessPerformBlock: Methode aussehen?

Wie sollte nun deine Check_File_Existence() Methode aussehen?

Jetzt integriert die Methode nur noch. Sie enthält keine Logik mehr, also keine Kontrollstrukturen und keine Ausdrücke. Sie fügt nur noch Teilfunktionalität (Operationen) zu einem neuen Größeren zusammen (Integration).

Prinzipiell gefällt mir diese Idee.

Nur eine Umsetzung nach Objective-C scheint mir nur schwerlich möglich.

Eventuell sollte ich mich in die Thematik der Closures noch etwas weiter einlesen, da diese immer weiter verbreitet und genutzt werden.

Ein kleiner Denkanstoß für die Check_File_Existence() Methode wäre aber auf jeden Fall noch hilfreich.

Dankende Grüße 

Marco

[Marcelo Emmerich]

Das Prinzip kann man auch in ObjC anwenden, allerdings bleibt dabei das Auslesen des Return-Wertes mit einer if-Abfrage bestehen:

- (void) appendString:(NSString*)string toFileWithPath:(NSString*)filePath {

    NSError *error = nil;
    BOOL writingSuccessful = [string writeToFile:filePath atomically:YES encoding:NSUTF8StringEncoding error:&error];
    if( !writingSuccessful )
        @throw [NSException exceptionWithName:@"IOException" reason:@"Problem writing to file" userInfo:@{@"error": error}];
}

Geht's anders besser? Wie würdet ihr das implementieren?

[Marco Feltmann]

Am Mittwoch, 30. Januar 2013 15:34:04 UTC+1 schrieb Don:

Und damit ist die Frage beantwortet, wohin was gehört.

Das schon, ja.

Bleibt immer noch die Frage nach dem "Wie?".

In Verbindung mit deinem ersten Beitrag wäre das Sinnvollste vermutlich so etwas wie:

        NSError *error = nil;

    [string writeToFile:filePath atomically:YES encoding:NSUTF8StringEncoding error:&error];

        [Communicator handleError:error];

Der Communicator trifft dann einfach die Entscheidung, ob das Fehlerobjekt gesetzt ist oder nicht.

Und je nachdem tut er damit Dinge oder eben nicht.

Übrigens: das Logging scheint mittlerweile überall Gang und Gäbe zu sein. Es ist beispielsweise unter Android sehr gern bei Exceptions genommen.

Ich finde das ebenfalls grauenhaft.

Doch mit deinem Tipp bin ich in der Lage den inhaltlichen Unterschied zu erkennen.

Das NSError Objekt existiert ja eben nur, um dem User mitzuteilen, dass da irgend etwas nicht so funktioniert hat wie geplant.

Also muss diese Mitteilung logischerweise von einer ganz anderen Instanz durchgeführt werden.

Sofern die Funktionalitäten strikt getrennt sind, sollte hier ja kein Folgefehler auftreten - ein Abbrechen des Vorgangs dürfte also überhaupt nicht notwendig sein.

Damit fällt die Auswertung des Rückgabewertes flach und ich bin meine Kontrollstrukturen los.

Ich bin gespannt, wie sich dieser theoretische Ansatz in der Praxis bewähren wird.

Vielen Dank für die hilfreichen Tipps!