{PHP} Richtiger Einsatz von Exceptions @CODEKICKER

Richtiger Einsatz von Exceptions

Ich habe folgenden Code bei einem Kollegen gesehen und bin mir nicht sicher ob er die Exception wirklich richtig einsetzt:

public function ($number, $text) {
  if(!is_number($number)) {
    throw new Exception('$number is not a valid number');
  }
}

oder eine Abwandlung davon (wenn die Methode nur true oder false zurück geben soll):

/**
 * @return boolean
 */
public function ($number, $text) {
  try {
    if(!is_number($number)) {
      throw new Exception('$number is not a valid number');
    }
  } catch(Exception $e){
    return false;
  }
}

Für mich sieht es nach Missbrauch der Exception für die Steuerung des Codes aus. Ist es das?
Wie ist die Best Practice dazu?

News:

07.02.2012

blackdog9

113 ● 1 ● 5

Editiert 08.02.2012

Floyd

14,6k ● 3 ● 9

		Es gab hier mal vor ein paar Monaten eine sehr ausführliche Kaffeeküchen-Diskussion (leider vor der Geburt der Lounge) über Best-Practice von Exception-Handling. Ich würde gerne einen Link setzen, finde aber die betreffende Frage nicht mehr.. :( Warum kann man eigentlich nicht durch alle eigenen Beiträge browsen? – puls200 17.02.2012
		Da gab es schon weit mehr als eine. Hast Du es schon mit der Site-Suche versucht? Oder erinnerst Du Dich noch an weitere Details, um die es da ging? – Matthias Hlawatsch 17.02.2012
		Habs wiedergefunden: http://codekicker.de/fragen/Klassenschrieben Wichtig bei der Site suche ist die Verknüpfung der Begriffe mit "+", an die man sich noch erinnern kann ;-) – puls200 17.02.2012

4 Antworten

Also der erste Code erscheint mir auf den ersten Blick so in Ordnung, aber der 2. Code da kräuseln sich mir die Zehennägel nach oben.
Zweiterer Codeblock macht in dieser Form aus meiner Sicht nur Sinn, wenn is_number selbst eine Exception werfen würde. Das erwarte ich aber von einer Funktion mit dem Namen nicht, denn diese sollte jedwede Exception intern schon selbst abfangen und false zurückgeben.
Insofern sollte der zweite Part eher so aussehen:

/**
 * @return boolean
 */
public function ($number, $text) {
  if(!is_number($number)) {
    return false;
  }
  else {
    return true;
  }
}

oder noch kürzer:

/**
 * @return boolean
 */
public function ($number, $text) {
  return is_number($number);
}

07.02.2012

Karill Endusa

1,5k ● 1 ● 9

		was passier mit den Zehennägel wenn man den zweiten Beispiel folgender massen abändert: [code] public function do($number, $text) { try { $db->beginTransaction(); if(is_number()) { throw new Exception('Invalid number'); } [...] $db->commit(); } catch(Exception $e) { $db->rollback(); throw $e; } } [/code] – blackdog9 07.02.2012
		Die kriegen noch einen Kringel mehr. Erst mal die Argumente prüfen: if(!is_number()) { throw new Exception('Invalid number'); } DANACH die eigentliche Arbeit tun: try { $db->beginTransaction(); ... } catch (...) {} – Matthias Hlawatsch 07.02.2012
		Die Kringel bekomm ich so schnell nich mehr raus... BTT: Als Ergänzung zu meiner Aussage _nachdem_ der Hinweis von Matthias befolgt wurde, möchte ich noch sagen, dass es bei dieser Lösung ja dann nich nur um den return geht. Hier wird ja die Exception gefangen, um einen Rollback durchzuführen, und danach eh wieder geworfen. Demnach darf do() sowieso einen Fehler werfen (und sollte es auch. Sobald ein boolscher Rückgabewert hinzukommt, sollte das nicht mehr der Fall sein (und der Name "tryDo()" würde besser passen)) – Karill Endusa 07.02.2012

                    Ich stehe (auch mangels Tags) gerade auf dem Schlauch, um welche Programmiersprache es hier geht. Ich tippe mal auf PHP. Soweit ich googlen kann, gibt es da die InvalidArgumentException. Ich finde, zu "Best Practice" gehört auch, möglichst spezifische Ausnahmen zu werfen (hier eben z.B. InvalidArgumentException) und niemals (außer evtl. an sehr wenigen, zentralen Stellen in der Anwendung) die Basis-Exception zu fangen. In dem Beispiel aus Deinem Kommentar zu Karills Antwort sollte deshalb besser stehen "catch (PDOException $ex)" oder etwas vergleichbares.
                    
                

07.02.2012

Matthias Hlawatsch

13,2k ● 4 ● 9

ja, es sollte PHP sein ;-) und ja, normalerweise müsste man den richtigen Typ Exception benutzen. Für den Beispiel habe ich es aber etwas "vereinfacht" ;-)
Ist aber ein berechtigter Einwand.

– blackdog9 08.02.2012

[gelöscht]

17.02.2012

DaSpors

4,2k ● 2 ● 8

Editiert 17.02.2012

DaSpors

4,2k ● 2 ● 8

-1

                    Nach Deinem Kommentar zum Post von Karill ("was passier mit den Zehennägel wenn man den zweiten Beispiel folgender massen abändert[...]") kann man antworten:

"Für mich sieht es nach Missbrauch der Exception für die Steuerung des Codes aus. Ist es das?"
Nein, ist es nicht. Es werden vorhandene Strukturen genutzt (DB Exceptions werden eh abgefangen, also wird das vorhandene Schema genutzt).

"Wie ist die Best Practice dazu?"
Für mich genau so :)

07.02.2012

DaSpors

4,2k ● 2 ● 8

Editiert 07.02.2012

DaSpors

4,2k ● 2 ● 8

7		-1: Wie man es im konkreten Fall besser machen kann, steht in meinem Kommentar zu blackdog9's Rückfrage. Und allgemein halte ich das Werfen von Exceptions, die noch in der selben Methode gefangen werden, für eine besonders schlecht lesbare Form des goto-Befehls und deshalb eher für worst als best practice. – Matthias Hlawatsch 07.02.2012
		dem kann ich mich nur anschließen: try-throw-catch ist einfach "Gänsehaut-Code"... nicht nur, aber eben auch! – Karill Endusa 07.02.2012
		@Matthias: Die goto-Assoziation ist ja mal richtig gut...+1 und danke für das Geschenk dieses Nachmittags-Schmunzlers... – ffordermaier 07.02.2012
		Es gibt bestimmt 100 verschiedene Möglichkeiten, warum das werden einer Exception innerhalb eines try blocks Sinn machen kann. Weil Ihr dabei eine Gänsehaut bekommt wird das nicht schlechter. Das sagt nur etwas über Euer Vorstellungsvermögen aus. Beipiele: Generalisiertes Logging, lesbare Kontrollstukturen, ... Nur weil dieses Beipiel einfacher zu lösen ist, ist es kein Bad Practice! Nehmt die Scheuklappen ab! – DaSpors 17.02.2012
		-werden +Werfen – DaSpors 17.02.2012

Stelle deine Php-Frage jetzt!

Kontakt, Impressum, Lizenz, AGB