What i'm doing wrong here?

Question

So, i'm working on building my first MVC based application.

I followed this tutorial: requiremind.com/a-most-simple-php-mvc-beginners-tutorial/ and now i'm creating a forum based on the above tutorial.

This is in my index.php

if(isset($_GET['controller']) && !isset($_GET['action'])) {
    $controller = $_GET['controller'];
    $action = 'topic';
} else {
    $controller = 'main';
    $action = 'index';
}

The first condition is for displaying pages with topic and will work for url with ?controller=PHP or something like this and set the $action as 'topic'.

Now the request will be transferred to routes.php

function call($controller, $action) {

    require_once('controllers/main_controller.php');

    if($controller == 'main') {
        require_once('models/main_model.php');
        $controller = new MainController();
    } else {
        require_once('models/topic_model.php');
        $controller = new MainController();
    }

    $controller->$action();
}

$db = Db::getInstance();
$query = $db->query('SELECT cat_name FROM category');
$categories = $query->fetchAll();

foreach($categories as $category_name) {
    if($controller == $category_name && $action == 'topic') {
        call($controller, $action);
    }
}

if(($controller == 'main') && ($action == 'index')) {
    call($controller, $action);
}

Now this will make a request with database and if the topic/category exists, it will call the function.

main_controller.php

class MainController {
    public function index() {
        $category = Main::home();
        require_once('views/home.php');
    }

    public function topic() {
        $topics = Topic::thread_topic($_GET['controller']);
        require_once('views/topic.php');
    }
}

After the main_controller.php, topic_model.php:

<?php

class Topic {
    public $thread_topic;
    public $thread_desc;
    public $thread_created_at;
    public $thread_created_by;
    public $category_id;

    public function __construct($thread_topic, $thread_desc, $thread_created_at, $thread_created_by, $category_id) {
        $this->thread_topic = $thread_topic;
        $this->thread_desc = $thread_desc;
        $this->thread_created_at = $thread_created_at;
        $this->thread_created_by = $thread_created_by;
        $this->category_id = $category_id;
    }

    public function thread_topic($controller) {
        $list = [];
        $cat_id = Fnct::cat_name_to_id($controller);
        $db = Db::getInstance();
        $stmt = prepare('SELECT * FROM thread WHERE thread_created_by =:cat_id');
        $stmt->execute(array(':cat_id' => $cat_id));
        foreach($stmt->fetchAll() as $thread) {
            $list[] = new Topic($thread['thread_topic'], $thread['thread_desc'], $thread['thread_created_at'], $thread['thread_created_by'], $thread['category_id']);
    }
        return $list[];
    }
}

?>

And now the views/topic.php

<table border='1'>

    <tr>
        <th>Thread Topic</th>
        <th>Thread Desc</th>
        <th>Thread Started At</th>
    </tr>

    <tr>
        <?php foreach($topics as $topic) { ?>
        <td><?php echo $topic->thread_topic; ?></td>
        <td><?php echo $topic->thread_desc; ?></td>
        <td><?php echo $topic->thread_created_at; ?></td>
        <?php } ?>
    </tr>

</table>

The problem is this, it is displaying blank page. And giving no error.

And i'm still struggling to figure out what is wrong here. Please tell me what i did wrong here??

Edit: In routes.php moves $controller = new MainController() under if statement. But still same blank page is displaying.

Answer 1

Well, your error is in your routes.php as mentioned above. I'll try to explain it here in an answer and also directly fix the mistakes you did.

Okay, lets start. First of all, lets ignore the call() function first and only look at the other part:

Your Code:

$db = Db::getInstance();
$query = $db->query('SELECT cat_name FROM category');
$categories = $query->fetchAll();

foreach($categories as $category_name) {
    if($controller == $category_name && $action == 'topic') {
        call($controller, $action);
    }
}

if(($controller == 'main') && ($action == 'index')) {
    call($controller, $action);
}

You should know that fetchAll() will always return an arrqay, not a string, thats why you can't compare the result directly with a string. Change the code to:

$db = Db::getInstance();
$query = $db->query('SELECT cat_name FROM category');
$categories = $query->fetchAll();

foreach($categories as $category_name) {
    if($controller == $category_name["cat_name"] && $action == 'topic') {
        call($controller, $action);
    }
}

if(($controller == 'main') && ($action == 'index')) {
    call($controller, $action);
}

You see the $categoriy_name["cat_name"] - You have to give the key, since $category_name is an array, not a string. (May look at some tutorials for PHP and SQL, like how all the fetch method works and what their output is).

And then the second part, your call function

Your code:

function call($controller, $action) {

    require_once('controllers/main_controller.php');
    $controller = new MainController();

    if($controller == 'main') {
        require_once('models/main_model.php');
    } else {
        require_once('models/topic_model.php');
    }

    $controller->$action();
}

So, you pass $controller as an argument to that function. Thats correct. But then you assign $controller = new MainController(); so $controller is no longer the string you passed to the function, but now ins an object of the class MainController. So you loose your value on what you passed to the function. And then only 1 Line after you do:

if($controller == 'main') {

This will never be true. At this point $controller is an object of your mainController() and the argument will always evaluate to false, so you'll always load the topic_model.php Since your topic() function is inside the MainController() its okay, but its not what MVC is for. Thats just.. some random code there. Thats not MVC, nor good practice.

But well, I don't know what you want to do there with your main controller, I'm not going to dive more into that topic. With the changes of your foreach-loop the call should work and your view should be loaded.

You don't want something like a MainController that is always laoded, or at least not for those methods you have inside - May look up some tutorials on MVC. You only wanna load the controller for the specific section you're in.

Anyway, I wish you all the best for the future and happy coding! :)