Is this a good way to process an order? (SQL+PHP)

Question

I'm making a simple webshop, it's not the full code, there are more security procedures. I'm just want to know your opinions. Thank you.

These SQL tables I have:

(I have more column in this table this is just an example)

customers:

id	name	phone	address	zip	city	country	o_date
1	Test	12345	Test Adress	1234	Test City	Test Country	01-02-2022

orders

customerid	orderid	orderdate	total_price
1	OS1	01-02-2022	19

orderitems

id	orderid	productid	item_price
1	OS1	P1	19

shipping

orderid	shipping_method	shipping_price	tracking_id	status	sent_date
OS1	courier	0	12345	sent	02-01-2022

products

product_id	productprice	currency	productqty	productname	weight
P1	19	19	USD	1	Test Product

You must have noticed that I have 3 times the price of the order/product in the tables. The reason why is in the products table I have the current product price this can be changed, and in the another tables I have the product price which was at the time of ordering.

After submitting the form the customer will land on this page:

order.php

/// Insert into customers table
    $data = [
        'name' => $name,
        'phone' => $phone,
        'adress' => $adress,
        'zip' => $zip,
        'city' => $city,
        'country' => $country,
        'o_date' => $date
    ];
    $sqlinsertcustomer = "INSERT INTO customers (id, name, phone, adress, zip, city, country, o_date ) VALUES ( '', :name, :phone, :adress, :zip, :city, :country, :o_date)";
    $stmt = $conn->prepare($sqlinsertcustomer);
    $stmt->execute($data);
    $last_id = $conn->lastInsertId();
    $order_id = "OS". $last_id;
    
    /// Get the total price from the actual product. 
    
    $stmtpprice = $conn->prepare("SELECT productprice FROM products WHERE product_id = :product_id;");
    $stmtpprice->execute([":product_id"=>$productid]);
    $productprice = $stmtsms->fetch();
    $totalpprice = $productprice["productprice"];
    
/// Insert into Orders, Orderitems, Shipping Table
    $stmtorders = $conn->prepare("INSERT INTO orders (customerid, orderid, orderdate, total_price) VALUES (:customerid, :orderid, :orderdate, :total_price);");
    $stmtorderitems = $conn->prepare("INSERT INTO orderitems (id, orderid, productid, item_price) VALUES ('', :orderid, :productid, :itemprice);");
    $stmtshipping = $conn->prepare("INSERT INTO shipping (orderid, shipping_method, shipping_price, tracking_id, status, sent_date) VALUES (:orderid, :shipping_method, :shipping_price, '0', '0', :sent_date);");
    $stmtorders->execute([":customerid"=>$last_id,
                          ":orderid"=>$order_id,
                          ":orderdate"=>$date],
                          ":totalprice"=>$totalpprice]);
    $stmtorderitems->execute([":orderid"=>$order_id,
                              ":productid"=>$productid],
                              ":itemprice"=>$totalpprice]);
    $stmtshipping->execute([":orderid"=>$order_id,
                            ":shipping_method"=>$shippingmethod,
                            ":shipping_price"=>$shippingprice,
                            ":sent_date"=>$date]);

What do you thing about processing and about the table structure? Thank you!

Answer 1

Some minor suggestions:

• I would move the code that retrieves the current product price to a dedicated function.
• The main code should be a standalone function too
• You should use a transaction to wrap up the whole sequence of operations, so that in case of error, you don't end up with inconsistent data and orphaned records.
• Separate the three blocks that write to tables Orders, Orderitems, Shipping Table.
• Add spacing where appropriate eg: $stmtorderitems->execute([":orderid" => $order_id,...
• Improve naming a little bit, for example $last_id should be $customer_id so that there is no misunderstanding.
• One potentially misleading statement: $totalpprice is the product price and not the total amount of the order as one might think. So just call it $product_price and there is no ambiguity.

---

$productprice = $stmtsms->fetch();
$totalpprice = $productprice["productprice"];

• The order ID is generated like this: $order_id = "OS". $last_id;. Why not simply use an ID as well ? A client may have more than one order...
• One problem in your code is that it requires horizontal scrolling, the SQL statement lines are too long. Wrap them up to have a better overview. I would perhaps do like this:

---

$stmtorders = $conn->prepare(
    "INSERT INTO orders (customerid, orderid, orderdate, total_price) 
    VALUES (:customerid, :orderid, :orderdate, :total_price);"
);

But you have considerable freedom here.

Answer 2

Quick remarks:

• Your naming should be more consistent. You sometimes use snake_case (item_price) and elsewhere you simply compound words (productid). (I'd prefer PascalCase, but perhaps in your DB snake_case is more common. I'm not a fan of simply compounding words, it makes things unclear.)

• Use descriptive column names, I have no idea what o_date is supposed to be.

• No need to repeat information: item_price should just be price, since each entry is an item (in an order). Same with product_id and productprice in the table products.

• I'm a fan of having an ID in each table, even if each entry could be identified by a combination of columns. To me a combination of columns feels less like an ID, and more like a constraint. (But that's an opinion, others will possibly/likely disagree.)

• Don't needlessly abbreviate: you gain nothing by using qty instead of quantity.