Rückgabewert angeben oder nicht

[MrChangelog]

Ich habe folgenden Code:

private function _insertGroupIdAndSizeIntoDB($index) {
    $fileId = $this->DbData[$index]['Id'];
    $groupId = $this->DbData[$index]['groupId'];
    $fileSize = $this->DbData[$index]['size'];        

    try {
        $this->PDO->beginTransaction();
        $updateCommand = $this->PDO->prepare('UPDATE `file` SET groupId = :groupId, filesize = :size WHERE fileId = :fileId;');
        $updateCommand->bindParam(':groupId', $groupId);
        $updateCommand->bindParam(':size', $fileSize);
        $updateCommand->bindParam(':fileId', $fileId);
        $updateCommand->execute();
        return true;
    } catch (PDOException $ex) {
        // aktive Transaktion rückgängig machen
        if ($this->PDO->inTransaction()) {
            $this->PDO->rollBack();
        }
        file_put_contents(self::$logFile, $ex->getMessage().' Vorgang wird abgebrochen!'."\r\n", FILE_APPEND);
        $this->hasError = true;
    }
}

$result = $this->_insertGroupIdAndSizeIntoDB(1);
if ($result) {
    echo 'Erfolg!';
}
// der Wert von $index ist für die Frage nicht relevant 

Wie man sieht wird

true 

zurückgegeben wenn die Sache erfolgreich war. Wenn eine Exception auftritt, gibt es keinen Rückgabewert. Beim debuggen (ich verwende NetBeans mit xDebugger) ist

$result 

dann nirgends zu finden aber

var_dump($result) 

gibt

null 

aus.

Funktionieren tut es so, aber ist es auch korrekt? Oder müsste ich im catch explizit 'return false' schreiben?

[php1704]

Entweder immer was zurückgeben oder nie.

Grüße.

[protestix]

Warum baust du zu Fuss ein Rollback. Im Handbuch steht unter PDO::ERRMODE_EXCEPTION

(Zur Erinnerung: Transaktionen bekommen automatisch einen Rollback, wenn eine Ausnahme das Script beendet.)

Funktionen sollten immer etwas zurückgeben in diesem Fall steht als Rückgabewert der Wert von execute zur Verfügung.

[hellbringer]

Wenn eine Funktion ihre Aufgabe nicht erfüllen kann, sollte sie eine Exception werfen.

[dlite]

Ich muss meinen Vorrednern Recht geben. Entweder du gibst im Catch-Block den Rückgabewert "false" zurück oder du reichst eine Exception aus der Funktion heraus, sodass klar erkennbar ist, dass es an der Stelle nicht korrekt weitergehen kann.
Schließen arbeitest du an den Rückgabewerten mit einem "variablen" Zustand eines Wertes weiter, unter der Prämisse, dass dieser Wert gültig ist.

Sauber wäre in meinen Augen, dass die Funktion eine personalisierte Exception wie z.B. TransmissionException geworfen und vom Funktionsaufrufenden abgefangen werden kann, sodass du eigtl. dass hier zur Überprüfung nimmst:

try
{
      $this->_insertGroupIdAndSizeIntoDB(1);
      echo 'Erfolg!';
}
catch(TransmissionException $transmissionExc){
     error_log("Could not transmit data to database");
} 

Das macht auch hinsichtlich der Codelesbarkeit wesentlich mehr Sinn, da direkt erkennbar ist, dass da was schief gehen kann.

[MrChangelog]

Warum baust du zu Fuss ein Rollback.

Weil eine PDOException hier das Skript eben NICHT beendet - und das auch nicht soll.

Wenn eine Funktion ihre Aufgabe nicht erfüllen kann, sollte sie eine Exception werfen.

Deswegen auch das "try ... catch"

[hausl]

Wenn eine Funktion ihre Aufgabe nicht erfüllen kann, sollte sie eine Exception werfen.

Deswegen auch das "try ... catch"

catch = fangen
throw = werfen -> throw new Exception()

[hellbringer]

Weil eine PDOException hier das Skript eben NICHT beendet - und das auch nicht soll.

Das hat eine Funktion aber nicht selber zu entscheiden, ob die Anwendung beendet werden soll. Deswegen gibt es ja Exceptions, damit man die Fehlerbehandlung nach außen delegieren kann. Die Zuständigkeit dafür liegt ja außerhalb der Funktion und nicht in der Funktion.

[MrChangelog]

Das hat eine Funktion aber nicht selber zu entscheiden, ob die Anwendung beendet werden soll.

Stimmt, das entscheide ich

[hellbringer]

Stimmt, das entscheide ich

Ja, durch Spaghetticode.

[protestix]

Wenn der Programmierer einen Fehler gemacht hat dann soll er davon Kenntnis erhalten.

Ob ein Fehler im Statement vorliegt entscheidest nicht du, sondern der Treiber der Datenbank, der als Feedback einen Fehler von der DB erhält.
Bevor das nicht an die DB geschickt wird, weiss ja erst mal keiner ob etwas nicht stimmt.

Nehmen wir an du hast dich verschrieben beim Namen der Tabelle und diese ist nicht vorhanden.
Dann erfährt der Treiber ja erst nach Ausführung als Feedback, das etwas nicht stimmt, es gibt eine Fehlernummer dazu.
Über diesern Datenbanktreiber hast du mit PDO einen Wrapper gestülpt der das irgendwie übersetzt und da es eine Klasse ist wird eine Exception geworfen.
An dieser Stelle ist aber das evtl. nötige Rollback längst eingeleitet worden, demnach von dir auch keine weitere Aktion mehr notwendig.
Über diesen Wrapper PDO Klasse, stülpst du nun noch deinen eigenen Eception-Handler, der die geworfene Exception abfängt und selber eine Exception produziert.

Anderes Szenario.
Dein SQL-Statement ist syntaktisch korrekt, das Statement kann jedoch nicht ausgeführt werden, da kein passender Datensatz gefunden werden kann.
In diesem Fall gibt es als Rückgabe false vom Treiber. Wenn das Statement erfolgreich ausgeführt wurde hingegen true.

Du hast hier also 2 Unterschiedliche Fehlerszenarien.

1. Korrektes Statement aber keine betroffenen Datensätzen, entsprechend normale Programmausführung, es obliegt dem Programmierer hier entsprechend zu verfahren.
2. Falsches Statement, weil Programmierfehler, welcher korrigiert gehört, dann Exception vom Treiber und Programmabruch, da hier eine Nachbearbeitung seitens des Programmieres erfolgen muss.

Ich hoffe ich konnte das einigermassen nachvollziehbar darstellen.