1
\$\begingroup\$

This is the simple back-end, albeit crude, that I made for a React Native mobile app project as a proof of concept. It's a basic CRUD for tracking supply inventory. The "shopping list" is items with a Buy qty > 0.

selectData() is a wrapper for a SELECT query using prepared statements:

  • Signature: selectData($columns, $table, $where, $fetchType)
  • fetchType is a switch/case to set the argument for fetchAll().
  • $stmt = $conn->prepare("SELECT $columns FROM $table WHERE $where");

It was easier for testing to use a remote MySQL database than to learn how to make/use a local database in React Native.

While I know that it works, does this even remotely count as a web API? How RESTful is it (or not)? Roast me if you must, but please explain it simply so I can learn.

After I created this, I started learning/using Laravel, but I haven't gotten into Laravel APIs yet.

        <?php
        if(isset($_GET['action'])) {
        include "initial.php"; //DB config, etc.

        if ($_GET['action'] === "getShoppingList") {
            echo json_encode(getShoppingList());
        }
        else if ($_GET['action'] === "getSingleItem") {
            echo json_encode(getSingleItem($_GET['id']), JSON_NUMERIC_CHECK);
        }
        else if ($_GET['action'] === "getAllItems") {
            echo json_encode(getAllItems());
        }
        else if ($_GET['action'] === "insert") {
            if (!isset($_GET['in_use'])) {
                echo json_encode(insertItem($_GET['itemName']));
            }
            else if (isset($_GET['in_use']) && isset($_GET['spare']) && isset($_GET['target'])) {
                echo json_encode(insertItem($_GET['itemName'], $_GET['in_use'], $_GET['spare'], $_GET['target']));
            }
        }
        else if ($_GET['action'] === "update") {
            if (isset($_GET['field'])) {
                echo json_encode(updateSingleItemField($_GET['id'], $_GET['field'], $_GET['value']));
            }
            else {
                echo json_encode(updateItem($_GET['id'], $_GET['in_use'], $_GET['spare'], $_GET['target']));
            }
        }
        else if ($_GET['action'] === "delete") {
            echo json_encode(deleteItem($_GET['id']));
        }
    }

    function getShoppingList(){
        $items = selectData("id, item_name as name, in_use_quantity as 'in_use', spare_quantity as spare, target_quantity as target, CEILING(target_quantity - in_use_quantity - spare_quantity) as buy, updated", "items", "(target_quantity - in_use_quantity - spare_quantity > 0) ORDER BY item_name ASC", "obj");
        return $items;
    }

    function getSingleItem($id){
        $item = selectData("id, item_name as name, in_use_quantity as 'in_use', spare_quantity as spare, target_quantity as target, FORMAT(CEILING(target_quantity - in_use_quantity - spare_quantity),2) as buy, updated", "items", "id = $id", "obj");
        return $item;
    }

    function getAllItems(){
        $items = selectData("id, item_name as name, in_use_quantity as 'in_use', spare_quantity as spare, target_quantity as target, FORMAT(CEILING(target_quantity - in_use_quantity - spare_quantity),2) as buy, updated", "items", "1 ORDER BY item_name ASC", "obj");
        return $items;
    }

    function insertItem($itemName, $inUseQuantity=0, $spareQuantity=0, $targetQuantity=0){
        $conn = loadDatabase();
        $query = "INSERT INTO `items` (`id`, `item_name`, `in_use_quantity`, `spare_quantity`, `target_quantity`, `updated`) VALUES (NULL, '$itemName', '$inUseQuantity', '$spareQuantity', '$targetQuantity', CURRENT_TIMESTAMP())";
        $stmt = $conn->prepare($query);
        $stmt->execute();

        $id = $conn->lastInsertId();
        $conn = null;
        return $id;
    }

    function updateSingleItemField($id, $field, $value){
        $conn = loadDatabase();
        $fieldName = $field . "_quantity";
        $stmt = $conn->prepare("UPDATE `items` SET `$fieldName` = $value WHERE `items`.`id` = $id");
        $stmt->execute();
        $conn = null;
        return $stmt->rowCount(); //returns TRUE if it was successful or FALSE if it failed
    }

    function updateItem($id, $inUseQuantity=0, $spareQuantity=0, $targetQuantity=0){
        $conn = loadDatabase();
        $stmt = $conn->prepare("UPDATE `items` SET `in_use_quantity` = $inUseQuantity, `spare_quantity` = $spareQuantity, `target_quantity` = $targetQuantity WHERE `items`.`id` = $id");
        $stmt->execute();
        $conn = null;
        return $stmt->rowCount(); //returns TRUE if it was successful or FALSE if it failed
    }

    function deleteItem($id){
        $conn = loadDatabase();
        $stmt = $conn->prepare("DELETE FROM `items` WHERE `items`.`id` = $id");
        $stmt->execute();
        $conn = null;
//returns TRUE if it was successful or FALSE if it failed
        return $stmt->rowCount();
    }
\$\endgroup\$
  • 1
    \$\begingroup\$ Just a personal style comment: I would rather use a switch if you've got no other criteria than the content of a variable. \$\endgroup\$ – csabinho Aug 27 at 5:00
5
\$\begingroup\$

Disclaimer: there will be a lot of links to my own site because I am helping people with PHP for 20+ years and got an obsession with writing articles about most common issues.

I don't know much about restful stuff or whether it does matter, but there are many other areas of improvement. Let's review some of them

The selectData() function

First of all, the selectData() function is a wrapper for a SELECT query using cargo cult prepared statements which makes it essentially insecure. It's a placeholder that makes your prepared query secure, not just a magical call to prepare().

Another thing is that gibberish selectData() syntax. Honestly, what are you trying to bargain here for? Spare yourself typing a few SQL keywords, seriously? Making it gibberish out of a precious almost-natural-English sentence, convenient and compatible? Look, you are already abusing the $where parameter by weird "1 ORDER BY item_name ASC" statement. Last but not least: such a syntax makes it harder to use prepared statements.
Come on. Let's keep it SQL.

Also, it seems that selectData() connects every time of it's own. Although it is not a problem for a primitive atomic API that always perform just a single SQL query per HTTP request, no API would remain thus. It will grow up to perform many queries per request and having each query to connect of its own will become a problem. Your script should always create just a single connection per single HTTP request and use it for all queries. So create a $conn variable as shown here and then use it for all database interactions in your code.

Also, I suppose that selectData() always returns a simple 2-dimensional list, which severely cripples the PDO's great ability to return the requested data in many different formats. So, instead make this function to return a PDO statement from which you'll be able to get the resulting data. So it will work as a sort of that "obj" thingy, but in a much more versatile way.

Given all the above, create a PDO helper function like this:

function pdo($pdo, $sql, $args = NULL)
{
    if (!$args)
    {
         return $pdo->query($sql);
    }
    $stmt = $pdo->prepare($sql);
    $stmt->execute($args);
    return $stmt;
}

So it keeps the SQL syntax intact yet makes all your queries safe and result sets versatile.

Let's rewrite some your functions to this new format

function getShoppingList($conn){
    $sql = "SELECT id, item_name as name, in_use_quantity as 'in_use',
               spare_quantity as spare, target_quantity as target, 
               CEILING(target_quantity - in_use_quantity - spare_quantity) as buy, 
               updated 
            FROM items 
            WHERE target_quantity - in_use_quantity - spare_quantity > 0
            ORDER BY item_name ASC";
    return pdo($conn, $sql)->fetchAll();
}

function getSingleItem($conn, $id) {
    $sql = "SELECT id, item_name as name, in_use_quantity as 'in_use', 
              spare_quantity as spare, target_quantity as target,
              FORMAT(CEILING(target_quantity - in_use_quantity - spare_quantity),2)
              as buy, updated 
           FROM items
           WHERE id = ?";
    return pdo($conn, $sql, $id)->fetch();
}

function insertItem($conn, $itemName, $inUseQuantity=0, $spareQuantity=0, $targetQuantity=0){
    $sql = "INSERT INTO `items` 
            (`id`, `item_name`, `in_use_quantity`, `spare_quantity`, `target_quantity`, `updated`) 
            VALUES (NULL, ?,?,?,?, CURRENT_TIMESTAMP())";
    pdo($conn, $sql, [$itemName, $inUseQuantity, $spareQuantity, $targetQuantity]);
    return  $conn->lastInsertId();
}

a couple notes:

  • getSingleItem() returns a single item, not a multidimensional array
  • although you can designate the row format as a parameter in fetch() or fetchAll(), it would be most convenient to set up the default fetch mode in the beginning. So you would only define it explicitly in case you will need some different format. Therefore n need for that ever-present "obj" parameter.
  • notice the brand new insertItem(). Neat, eh?
  • all your queries are secure now

Now you can rewrite all other functions to this format.

The router part

There are some repetitions in the router part that could be optimized

switch ($_GET['action']) {
     case "getShoppingList":
        $response = getShoppingList();
        break;

    case "getSingleItem":
        $response = getSingleItem($_GET['id']);
        break;

    case "getAllItems":
        $response = getAllItems();
        break;

    case "insert":
        if (!isset($_GET['in_use'])) {
            $response = insertItem($_GET['itemName']);
        } elseif (isset($_GET['spare'], $_GET['target'])) {
            $response = insertItem(
                $_GET['itemName'], $_GET['in_use'], $_GET['spare'], $_GET['target']
            );
        }

    case "update":
        if (isset($_GET['field'])) {
            $response = updateSingleItemField($_GET['id'], $_GET['field'], $_GET['value']));
        } else {
            $response = updateItem(
                $_GET['id'], $_GET['in_use'], $_GET['spare'], $_GET['target']
            );
        }

    case "delete":
        $response = deleteItem($_GET['id']);

}
echo json_encode($response);

The code formatting

In your question, one has to scroll A LOT to read the code. And in my answer you have all the code visible. Please do not torture yourself or anyone else by making that awhul horizontal scrollbar.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.