0

I'm trying to practice OOP but with native PHP.

I have my 'controller', My_Controller.php:

session_start();
if (!isset($_SESSION['userId'])) exit('You are not authorized to access this page');

// ... some code ...

if(isset($_GET['action']))
{
    switch($_GET['action']) {
        case 'getOrder':
            if(isset($_GET['id'])) {
                $orderDetails = $jobModel->getOrderById($_GET['id']);
                    header('Location: order-details.php');
            }
            break;
        default:
            echo 'Invalid action';
            break;
    }
}

And this is my 'view', order-details.php:

<?php
require_once './My_Controller.php';
?>
<html>
    <head>
        <title>Order Details</title>
    </head>

<body>
    <div>
        <a href="order-list.php">Back to Order List</a>
    </div>
    <div>Order Details</div>
    <div>
        <form id="form-add-job-item" method="post" action="">
            <table border="1">
                <thead>
                    <tr>
                        <th>Item Name</th>
                        <th>Quantity</th>
                        <th>Amount</th>
                    </tr>
                </thead>
                <tbody>
                    <?php
                        if(isset($orderDetails) && $orderDetails != 0) {
                            foreach($orderDetails as $orderItem => $value) {
                                ?>
                                <tr>
                                    <td><?= $value->name; ?></td>
                                    <td><?= $value->quantity; ?></td>
                                    <td><?= $value->amount; ?></td>
                                </tr>
                                <?php
                            }
                        }
                    ?>
                </tbody>
            </table>
            <button type="submit">Add Item</button>
        </form>
        <?php

        ?>
    </div>
</body>
</html>

order-details.php is some sort of a template to display the information for every order depending on the contents of $orderDetails.

It is called via separate page containing a table of orders. Each order in the table has a link:

<tr>
    <td><a href="My_Controller.php?action=getOrder&id=<?= $value->job_id; ?>"><?= $value->job_id; ?></a></td>
    <td><?= $value->job_date; ?></td>
    <td><?= $value->total_amount; ?></td>
</tr>

This is so it can be dynamic, in that I won't have to code a separate page for each order. This template will just hold variables and those variables will be filled with the relevant information based on the passed order ID, which will depend on what link the user clicked.

WHAT I NEED TO ACCOMPLISH:

I need to access the contents of $orderDetails and show the list of order items in order-details.php but I'm not sure how to do that? With what I have so far, I get a NULL value from the $orderDetails variable when accessing it from order-details.php.

I have checked the results from the database query using var_dump($orderDetails) and it does return the expected results.

UPDATE:

inside My_Controller.php:

case 'getOrder':
      if(isset($_GET['id'])) {
        // $dba contains the connection to the database
        $MyController = new My_Controller($dba);
        $MyController->getOrderById($_GET['id']);
      }
      break;

// ... Some code ...

class My_Controller
{
    private $myModel;

    public function __construct(Db $db)
    {
        $this->myModel = new My_Model($db);
    }

    public function getOrderById($orderId)
    {
        $orderDetails = $this->myModel->getOrderById($orderId);
        include './order-details.php';
    }
}
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
herondale
  • 729
  • 10
  • 27
  • 1
    `$orderDetails` is NULL because on your controller you are redirecting to another page therefore making a new request. And every new request resets everything. – Raphael Cunha Nov 19 '17 at 02:55
  • header('Location: order-details.php'); why you redirected? Redirecting this page will destroy the variable, alternatively you can store it as a session – Farsheel Nov 19 '17 at 02:57

1 Answers1

2

That variable will be accessible without doing anything special because it is in global scope. In other words, you can just access it as $orderDetails.

The trick is that it has to be defined. The way your code in My_Controller.php is set up, $_GET['action'] must be equal to getOrder and $_GET['id'] must be defined, or $orderDetails will not be set.

Here's the catch: this line of code ensures that $orderDetails is never set when you get to your display logic:

header('Location: order-details.php');

This redirect doesn't preserve the $_GET parameters. It triggers a brand new request with no parameters. So, after the redirect, your logic loading the order details never runs.

As for how to solve it: that depends on what you're trying to do, but most likely you shouldn't have that redirect there at all.

Also, you should know that using lots of global variables like this is considered bad practice. You should start breaking your code into small, reusable chunks using functions or objects.

elixenide
  • 44,308
  • 16
  • 74
  • 100
  • I see. What approach do you suggest I do? I'm basically trying to imitate the MVC behavior, wherein in the Controller, you can pass data back to it from the Model, and pass that data along to the View, and then access the data in the View. I'm basically trying to avoid having to manually code and create a separate page for orders, so I set up a template, and depending on the passed order ID, it will contain the information about that order. – herondale Nov 19 '17 at 03:02
  • Well, a controller should actually control the flow of execution. Right now, it doesn't really. I agree that it looks like it does, but it's doing a redirect and trying to punt on further decision-making. It *should* be deciding what code to include in the first place. As for how to fix it: like I said, you probably don't even need or want that redirect in the first place. Without more of your code it's hard to be sure, but removing that line may be all you need to do. – elixenide Nov 19 '17 at 03:08
  • I need the redirect because the Controller is first called in that separate page containing the table of orders, wherein each order has a link attached that redirects to the Controller first so it can load the information about that order from the database, and then display that loaded information in a separate page. Order List Page -> Click on an order -> redirect to Controller -> load data from database for clicked order -> Go to a separate page containing the data from database for clicked order – herondale Nov 19 '17 at 03:12
  • Using a redirect for that is the wrong approach, though. Instead, in this instance, the controller should simply be running the logic to load the required information for each view. So it would run one set of logic when called to load a table of orders, but different logic for details about an order. Part of your problem is that you're trying to use MVC, which is a concept in object-oriented programming (OOP), without doing OOP. Square peg, round hole. – elixenide Nov 19 '17 at 03:15
  • I see. The truth is I've had experience using CodeIgniter, but I don't know yet the nitty-gritty regarding how to accomplish the part where you include the Controller class name in the URL and the function inside it that you need and voila, it 'redirects', etc. If I create My_Controller as a class, I would have to instantiate it, but then I'm not sure where to go from there if I'm trying to accomplish what I'm trying to accomplish here. – herondale Nov 19 '17 at 03:26
  • @herondale is ’My_Controller.php’ being called everywhere, or just when the action ’getOrder’ is called? – Raphael Cunha Nov 19 '17 at 03:28
  • @herondale Break the logic into chunks, and only run the chunks that are appropriate based on your context. E.g., `$controller->loadAllOrders()` or `$controller->loadOrderDetails($_GET['id'])`. Don't rely on a redirect to bail out and stop executing the logic after that point. – elixenide Nov 19 '17 at 03:30
  • @RBCunhaDesign since I'm trying to make it behave like a Controller for orders, it is called for any action related to orders, like for inserting an order or viewing an order. So I guess not totally everywhere, but it is definitely called in more than one place, as I also have a button for inserting an order. – herondale Nov 19 '17 at 03:36
  • @EdCottrell I already get your point, I think my main problem is how will I be able to pass the data that I loaded from the Model class that was passed back to the Controller, to a separate or new View. I've been able to do that in CodeIgniter, where I load data from a Model, which gets passed back to the Controller, which then can be passed to the View. The Controller functions can be called using `` and that I don't know how to do yet in native PHP, make a class or object function like a link. That's why I kind of resorted to using a 'pseudo-Controller'. – herondale Nov 19 '17 at 03:42
  • @herondale If you're loading it the way you've shown, you don't have to pass it at all because you're doing everything in global scope. It's already available, no passing required. Again, that's not a good idea, but it's where your code is right now. – elixenide Nov 19 '17 at 03:52
  • @EdCottrell I figured out a way, but I'm not sure yet if it's a good one. So basically I still have all that switch statement above in my question, but inside `getOrder` case, I instantiate a `My_Controller` class which is in the same file but right below all that, which has a `getOrderDetails($orderId)` function, and inside it, I make a call to the Model, and then I just `include './order-details.php'`. Is this right? or should I look for another way to do it? – herondale Nov 19 '17 at 04:16