HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Return a tuple from MySQL database with statements

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
returnwithstatementsdatabasetuplemysqlfrom

Problem

I have a database from which I'd like to return a tuple of two elements (ID and URL) with check digit to see if operation was successful.

Is correct this code to return this tuple and control?

public function getManufacturer($id) {
    $stmtManufacturer = $this->dbConnection->prepare("SELECT ID, URL FROM ERP.MANUFACTURER WHERE ID = ?");
    $stmtManufacturer->bind_param("i", $id);
    $stmtManufacturer->execute();
    $stmtManufacturer->store_result();
    $stmtManufacturer->bind_result($id, $url);
    $stmtManufacturer->fetch();
    $stmtManufacturer->close();
    return [
                ACTION_RESULT => $this->processServerResponse($stmtManufacturer),
                "data" =>
                [
                    "id" => $id,
                    "url" => $url
                ]
            ];
}


ACTION_RESULT is defined as a constant in the class.

I think function processServerResponse() could be improved:

public function processServerResponse($stmt) {
    if ($stmt->errno || $stmt->affected_rows === 0) {
        $_actionResult = 0;
    } else if ($stmt->affected_rows !== 0) {
        $_actionResult = 1;
    }
    return $_actionResult;
}

Solution

The whole processServerResponse() method looks rather alien to me. I never seen anything like that. A server response shouldn't be so much tightly coupled with such a specific stuff like a statement class for one of PHP extensions. A server response should be from a server, not a statement.

Besides, you are mixing two response codes here, "error occurred" and "zero results found". It should be different codes.

Usually, errors are checked by a distinct error handler, while number of results found can be checked within the same routine that returns the data.

So I would make it this way

public function getManufacturer($id) {
    $stmt = $this->dbConnection->prepare("SELECT ID, URL FROM ERP.MANUFACTURER WHERE ID = ?");
    $stmt->bind_param("i", $id);
    $stmt->execute();
    $stmt->store_result();
    $stmt->bind_result($id, $url);
    $found = $stmt->fetch();
    return [
                ACTION_RESULT => (int)$found,
                "data" =>
                [
                    "id" => $id,
                    "url" => $url
                ]
            ];
}


while errors should be checked with a site-wide handler like this (taken from my article on writing Db wrappers)

set_error_handler("myErrorHandler");
function myErrorHandler($errno, $errstr, $errfile, $errline)
{
    error_log("[$errno] $errstr in $errfile:$errline");
    header('HTTP/1.1 500 Internal Server Error', TRUE, 500);
    header('Content-Type: application/json');
    echo json_encode(["error" => 'Server error']);
    exit;
}


So this way you will get different responses for the different scenarios.

On a side note, an obligatory suggestion: consider to use PDO instead of mysqli. This alone will do a huge refactoring for your code:

public function getManufacturer($id) {
    $stmt = $this->pdo->prepare("SELECT ID id, URL url FROM ERP.MANUFACTURER WHERE ID = ?");
    $stmt->execute([$id]);
    $data = $stmt->fetch();
    return [
                ACTION_RESULT => (int)(bool)$data,
                "data" => $data;
            ];
}

Code Snippets

public function getManufacturer($id) {
    $stmt = $this->dbConnection->prepare("SELECT ID, URL FROM ERP.MANUFACTURER WHERE ID = ?");
    $stmt->bind_param("i", $id);
    $stmt->execute();
    $stmt->store_result();
    $stmt->bind_result($id, $url);
    $found = $stmt->fetch();
    return [
                ACTION_RESULT => (int)$found,
                "data" =>
                [
                    "id" => $id,
                    "url" => $url
                ]
            ];
}
set_error_handler("myErrorHandler");
function myErrorHandler($errno, $errstr, $errfile, $errline)
{
    error_log("[$errno] $errstr in $errfile:$errline");
    header('HTTP/1.1 500 Internal Server Error', TRUE, 500);
    header('Content-Type: application/json');
    echo json_encode(["error" => 'Server error']);
    exit;
}
public function getManufacturer($id) {
    $stmt = $this->pdo->prepare("SELECT ID id, URL url FROM ERP.MANUFACTURER WHERE ID = ?");
    $stmt->execute([$id]);
    $data = $stmt->fetch();
    return [
                ACTION_RESULT => (int)(bool)$data,
                "data" => $data;
            ];
}

Context

StackExchange Code Review Q#157152, answer score: 3

Revisions (0)

No revisions yet.