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!

• sibert: In most of the tables you are consistent. The column names are unique. Why two columns "id" and not "item_id" and "cust_id". It is more of being consistent than anything else. I have found unique column names being easier to manage.

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.