Success page + data entry for a store page

Question

The purpose of this code is to log the inputs of a form (which is a menu for a shop) to the database to be kept as records as well as printed out as a receipt at the store.

I have been learning how to code from the internet and tried to put together my knowledge in a project. The code is working, but I would like to adopt elegance and good coding practices.

<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>Untitled Document</title>
<?php
require 'databaseaccess.php';
?>
</head>

<body>
<?php
//check order present
if(!($_POST))
{
    echo "error, order was not received!";
    echo "<br>";
    echo "return to <a href='orderformwscart.html'> order page </a>";

}
else{
    $total = 0;
    $soba_size = $_POST['soba_size'];
    switch($soba_size){
    case "small":
    $total = $total + 4;
    break;
    case "reg":
    $total = $total + 6;
    break;
    case "large":
    $total = $total + 8;
    break;
    }
    $soba_type = $_POST['soba_type'];
    $dbtoppings = '';
    if ($_POST['topping']){
        foreach($_POST['topping'] as $topping){
            switch($topping){
            case "duck":
            $total = $total + 1;
            $dbtoppings = $dbtoppings.",".$topping;
            break;
            case "egg":
            $total = $total + 2;
            $dbtoppings = $dbtoppings.",".$topping;
            break;
            case "chicken":
            $total = $total + 3;
            $dbtoppings = $dbtoppings.",".$topping;
            break;
            case "teriyaki chicken":
            $total = $total + 4;
            $dbtoppings = $dbtoppings.",".$topping;
            break;
            }
        }
    }
    if($_POST['requests']){
    $request = $_POST['requests'];
    }
    else{
    $request = '';
    }
//begin data entry
$data1 = array('total' => $total);
$STH = $DBH->prepare("INSERT INTO orderlist(total) values (:total)");
$STH->execute($data1);
$lastId = $DBH->lastInsertId();
//dataentry2
$data2 = array('orderid' => $lastId, 'soba_size' => $soba_size, 'soba_type' => $soba_type, 'topping' => $dbtoppings,'request' => $request);
$STH = $DBH->prepare("INSERT INTO orderdetails(orderID,soba_size,soba_type,topping,request) values (:orderid,:soba_size,:soba_type,:topping,:request)");
$STH->execute($data2);
}
?>

<div class="jumbotron">
        <h2 class="text-center">Thank you for ordering. Your order has been printed.
        <p id ="time">
        </p> </h2>
        <h3 class="text-center">Order Number: <?php echo $lastId; ?>
        </br>
        Amount:$ <?php echo $total;  ?>  </h3>
        <h4 class="text-center"><small>Please show this page at the cashier to confirm your order. Thank you!</small></h4>   
</div>
</body>
</html>

One fast comment: instead of putting almost everything into one, big else, you can just add return; as the last line of if(!($_POST))'s body. Or even better: Move that if to the very top of the file and change it's body to header('Location: orderformwscart.html'); exit; — Przemek, Commented Feb 17, 2016 at 11:55
Did not realise that I could have done it that way. Saves a layer of indentation! thanks! — Learnerng, Commented Feb 22, 2016 at 3:29

OffTheBricks · Accepted Answer · 2016-02-17 21:15:56Z

First and most important thing is to indent your code. Indenting provides an instant visual structure which makes it easy to read. More often than not, bugs and quirks can be easily avoided simply by good code indentation. PHP is especially susceptible to this because its code is mixed directly with the HTML output. There is actually an issue in your posted code where should there be an error, your code will still try to display the order details at the bottom.

Next thing you want to consider is creating your code in blocks. On a high level this is basic MVC code structure where the logic and the output are separated. You want to apply this as often as possible to make your code easier to read and maintain.

Make sure to close your database connection after opening it. I typically declare my database object outside the included class file in order to make it clearer what the variable is. This also helps your IDE out for providing popup advice, and code completion. If you're not using an IDE yet, try NetBeans or CodeLobster for PHP development.

Here's my take on your code. I obviously couldn't test it without your database details but it should give you some ideas.

<?php
class OrderDetails{
    private $Number;
    private $Amount;
    private $db;

    public function OrderDetails(){
        require 'databaseaccess.php';
        $this->db = $DBH;       //should probably declare $DBH outside the included file to make code easier to read, and to help the ide

        $this->ProcessOrder();

        //close database connection here
    }

    public function getNumber(){
        return $this->Number;
    }

    public function getAmount(){
        return $this->Amount;
    }

    private function ProcessOrder(){
        $total = 0;
        $soba_size = $_POST['soba_size'];
        switch($soba_size){
            case "small":
                $total = $total + 4;
                break;
            case "reg":
                $total = $total + 6;
                break;
            case "large":
                $total = $total + 8;
                break;
            default:break;      //always have a default even if you're confident it won't be needed
        }
        $soba_type = $_POST['soba_type'];
        $dbtoppings = '';
        if ($_POST['topping']){
            foreach($_POST['topping'] as $topping){
                switch($topping){
                    case "duck":
                        $total = $total + 1;
                        $dbtoppings = $dbtoppings.",".$topping;
                        break;
                    case "egg":
                        $total = $total + 2;
                        $dbtoppings = $dbtoppings.",".$topping;
                        break;
                    case "chicken":
                        $total = $total + 3;
                        $dbtoppings = $dbtoppings.",".$topping;
                        break;
                    case "teriyaki chicken":
                        $total = $total + 4;
                        $dbtoppings = $dbtoppings.",".$topping;
                        break;
                    default:break;      //always have a default even if you're confident it won't be needed
                }
            }
        }
        if($_POST['requests']){
            $request = $_POST['requests'];
        }
        else{
            $request = '';
        }

        //begin data entry
        $data1 = array('total' => $total);
        $STH = $this->db->prepare("INSERT INTO orderlist(total) values (:total)");
        $STH->execute($data1);
        $lastId = $this->db->lastInsertId();
        //dataentry2
        $data2 = array('orderid' => $lastId, 'soba_size' => $soba_size, 'soba_type' => $soba_type, 'topping' => $dbtoppings,'request' => $request);
        $STH = $this->db->prepare("INSERT INTO orderdetails(orderID,soba_size,soba_type,topping,request) values (:orderid,:soba_size,:soba_type,:topping,:request)");
        $STH->execute($data2);

        //set the class variables for grabbing by your other code
        $this->Amount = $total;
        $this->Number = $lastId;
    }
}
?><!DOCTYPE>
<html>
<head>
    <meta charset="utf-8">
    <title>Untitled Document</title>
</head>
<body>
    <?php
    //check order present
    if(!($_POST))
    {
        ?>
        error, order was not received!
        <br/>
        return to <a href='orderformwscart.html'> order page </a>
        <?php
    }
    else{
        $orderObj = new OrderDetails();
        ?>
        <div class="jumbotron">
            <h2 class="text-center">
                Thank you for ordering. Your order has been printed.
                <p id="time"></p>
            </h2>
            <h3 class="text-center">
                Order Number: <?php echo $orderObj->getNumber();?>
                <br/>
                Amount: $ <?php echo $orderObj->getAmount();?>
            </h3>
            <h4 class="text-center">
                <small>Please show this page at the cashier to confirm your order. Thank you!</small>
            </h4>   
        </div>
        <?php
    }
    ?>
</body>
</html>

Hi thanks alot for your feedback! I really like the idea of using classes and methods but I haven't managed to dive into it yet! For the error handling, I believe you are referring to the html output where i call a echo variable? What's the difference between the output between calling $lastId and calling the method of orderObj? I have been running all my code through dreamweaver which has been working so far, but it seems like a good idea to try out all sorts of IDEs. Again, thanks for your feedback, it has been great help to me in improving my current and future codes! =) — Learnerng, Commented Feb 22, 2016 at 3:26
Glad I could help. There's not functional difference between the way you're calling $lastId and the way I proposed with an orderObj, at least from the user's point of view. The code structure I proposed just makes the code easier to read, debug, and maintain. — OffTheBricks, Commented Feb 22, 2016 at 16:07

Stack Exchange Network

Success page + data entry for a store page

1 Answer 1

Your Answer

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

Hot Network Questions

Success page + data entry for a store page

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

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

Related

Hot Network Questions

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