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

Web application for a restaurant

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

Problem

I'm making a web application for a restaurant kind of type. The idea is to administrate the orders and customers.

For selecting one of the orders and showing more specific data about it, I have this PHP script. As you can see I am using prepared statements to prevent SQL injection.

setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

    if (!empty($_GET['order_id'])) {
        $order_id = $_GET['order_id'];

        $order_data = $connection->prepare("SELECT c.first_name, c.last_name, c.email_adress, c.customer_info, o.order_info, o.total_price, o.location, o.created FROM customers AS c LEFT JOIN orders AS o ON c.id = o.customer_id WHERE o.id = :order_id LIMIT 1");
        $order_data->bindParam(":order_id", $order_id, PDO::PARAM_INT);
        $order_data->execute();

        $query = "SELECT `products`.`name`, `orders-items`.`quantity` FROM `orders-items`" . "INNER JOIN `products` ON `orders-items`.`products_id` = `products`.`id`" . "WHERE order_id = :ordero_id LIMIT 1";
        $order_items = $connection->prepare($query);

        $order_items->bindParam(":ordero_id", $order_id, PDO::PARAM_INT);
        $order_items->execute();

        $orderObject = array();
        $orderObject['header'] = $order_data->fetch();
        $orderObject['items'] = array();
        while ($orderedItem = $order_items->fetch()) {
            $orderObject['items'][] = $orderedItem;
        }

        header('Content-type: application/json');
        echo json_encode($orderObject);

        $connection = null;
    }
} catch (PDOException $e) {
    echo $e->getMessage();
    die();
}


The parameters for the 2 queries are both the same. But I don't know how to use only one line for them.

  • The first query is for selecting the specific data about the order.



  • The second query is for selecting the items inside the order.



Both queries should be run, to get all results.

Problems

  • It's messy that I actually have 2 queries.



  • It's messy that I'm using 2 lines for the same

Solution

Aliases

Table aliases are handy, sure. But single-letter aliases are not good. It's OK to want to save having to type more characters than needed, but you have to keep in mind that things like aliases and variables get really confusing if the name you give it does not say anything about what it means. c, o, p, ot... Why not instead cust, ord, prod, ordItems?

Table naming

orders-items is not a good table name. Why? Well, because you will have to use back ticks each time you reference it, as opposed to your other tables. In SQL, avoid using reserved characters in table/column names to negate this problem. If possible, rename to orders_items or OrderItems or something along those lines.

To rename the table:

RENAME TABLE `orders-items` TO orders_items;


Consistency

Your SQL queries, though both are valid, look completely different. Compare this:

$order_data = $connection->prepare("SELECT c.first_name, c.last_name, c.email_adress, c.customer_info, o.order_info, o.total_price, o.location, o.created FROM customers AS c LEFT JOIN orders AS o ON c.id = o.customer_id WHERE o.id = :order_id LIMIT 1");


And this:

$query = "SELECT `products`.`name`, `orders-items`.`quantity` FROM `orders-items`" . "INNER JOIN `products` ON `orders-items`.`products_id` = `products`.`id`" . "WHERE order_id = :ordero_id LIMIT 1";


One of them you use table aliases, the other you use full table names. The latter you use back ticks, but not the former. The latter you also have periods splitting your query code. Stick to one style to make your code easier to maintain.
Your question

Your first query is selecting one specific order along with the customer associated with it. Your second query is selecting one specific order along with the items associated with it. If you want to combine both, then assuredly you will get customer information multiple times in your result set. Your LIMIT 1 seems like it would not be very useful... Can an order be placed by multiple customers?

However, since orders is your primary table, I suggest to start from there, then JOIN your other tables with INNER JOIN so you don't return nulls (if that is the intent, and assuming an order has to have a customer, and has to have items).

All said, I think this should work for what you are trying to do:

SELECT 
    ord.id,
    ord.order_info,
    ord.total_price,
    ord.location,
    ord.created,
    cust.id,
    cust.first_name,
    cust.last_name,
    cust.email_adress,
    cust.customer_info,
    ordItems.quantity,
    ordItems.products_id,
    prod.name,
    prod.price
FROM
    orders AS ord
        INNER JOIN
    customers AS cust ON ord.customer_id = cust.id
        INNER JOIN
    `orders-items` AS ordItems ON ord.id = ordItems.order_id
        INNER JOIN
    products AS prod ON ordItems.products_id = prod.id
WHERE
    ord.id = :order_id;

Code Snippets

RENAME TABLE `orders-items` TO orders_items;
$order_data = $connection->prepare("SELECT c.first_name, c.last_name, c.email_adress, c.customer_info, o.order_info, o.total_price, o.location, o.created FROM customers AS c LEFT JOIN orders AS o ON c.id = o.customer_id WHERE o.id = :order_id LIMIT 1");
$query = "SELECT `products`.`name`, `orders-items`.`quantity` FROM `orders-items`" . "INNER JOIN `products` ON `orders-items`.`products_id` = `products`.`id`" . "WHERE order_id = :ordero_id LIMIT 1";
SELECT 
    ord.id,
    ord.order_info,
    ord.total_price,
    ord.location,
    ord.created,
    cust.id,
    cust.first_name,
    cust.last_name,
    cust.email_adress,
    cust.customer_info,
    ordItems.quantity,
    ordItems.products_id,
    prod.name,
    prod.price
FROM
    orders AS ord
        INNER JOIN
    customers AS cust ON ord.customer_id = cust.id
        INNER JOIN
    `orders-items` AS ordItems ON ord.id = ordItems.order_id
        INNER JOIN
    products AS prod ON ordItems.products_id = prod.id
WHERE
    ord.id = :order_id;

Context

StackExchange Code Review Q#61691, answer score: 12

Revisions (0)

No revisions yet.