current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 8 down vote favorite

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've got this PHP script. As you can see i'm using prepared statements to prevent SQL injection.

try {
    $connection = new PDO('mysql:host=localhost;dbname=broodjes-service;charset=utf8mb4', 'root', 'password');
    $connection->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 parameter

Explanation; why there are 2 queries

Whenever I use one query like this:

SELECT c.first_name,
        c.last_name,
        c.email_adress,
        c.customer_info,
        o.order_info,
        o.total_price,
        o.location,
        o.created,

        p.name,
        ot.quantity

FROM customers AS c
LEFT JOIN orders AS o ON c.id = o.customer_id
LEFT JOIN `orders-items` AS ot ON o.id = ot.order_id
LEFT JOIN `products` AS p ON ot.products_id = p.id

WHERE order_id = :order_id;

I get 3 times the results of the specific data from the customer. Then I don't know how to get back the orders-items separately. Also, whenever there are no results, I have no idea how to 'not select them' within MySQL.

Also, when using that query, when a customer doesn't have any orders_items no result is given.

share|improve this question
2  
I have cleared all previous comments which were not (when combined), helpful. This question has working code, and the asker is looking for a less messy way to do it. This is clearly on-topic here. –  rolfl♦ yesterday

1 Answer 1

up vote 6 down vote accepted

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;
share|improve this answer
    
First of all, thanks for your review.But there is one problem with this mysql query. For example, there is an order wich has no orders-items in it. (for any reason, should not be possible but hey) And i run that query on that specific order_id, it doesnt return any rows. While i maybe want the name and other stuff about that customer? –  Bas yesterday
1  
@Bas You want a LEFT OUTER JOIN, which produces a row even when only the left table has an entry. –  200_success♦ yesterday
    
@Bas good point, I could see where a customer would walk in, want to order something, begin an order, and then up and leave without actually ordering something; you would still want that to be on the record. You could LEFT JOIN orders-items and products instead of INNER JOIN. –  Phrancis yesterday
    
@Phrancis I'd buy it. Is that serious btw? –  Bas yesterday
    
Yes it is serious. LEFT JOIN will return nulls while INNER JOIN won't. So in the case I stated, you would get something like order.id 01 | customer.id 01 | orders-items null | products null if that makes any sense. –  Phrancis yesterday

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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