0

I'm spending hours trying to solve this problem. My website worked well but i needed to change some ugly URLs.

I tried to implement a router (AltoRouter) and everything worked just fine after some changes until i got this error:

Fatal error: Uncaught Error: Call to a member function prepare() on null in foldername line X

I have a basic MVC, the Model looks like this:

Basically $pdo is null in this method: (And every single method in that class aswell)

class UsersModel {
    public function confirmedUser($username) {
        global $pdo;

        $sql = "SELECT * FROM users WHERE username = ? AND confirmed_at IS NOT NULL";
        $request = $pdo->prepare($sql); // Fatal error here
        $request->execute([$username]);
        return $request->fetch();
    }
}

The controller looks like this:

<?php
require_once "../php/connect.php";
require_once "../models/UsersModel.class.php";

$usersModel = new UsersModel();

if(array_key_exists('register', $_POST) && !empty($_POST)) {
    $username = $_POST['username'];
    $email = $_POST['email'];
    $password = $_POST['password'];
    $passwordConfirm = $_POST['passwordConfirm'];

    $usersModel->checkName($username);
    $usersModel->checkEmail($email);
    $usersModel->checkPassword($password, $passwordConfirm);
}

And my connect file:

<?php

const DBNAME = "imparfait";
const DBUSER = "root";
const DBPASS = "";

$pdo = new PDO('mysql:host=localhost;dbname='.DBNAME.';charset=utf8', DBUSER, DBPASS);

$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

The router looks like this: (a bit messy atm)

$router = new AltoRouter();

// map routes
$router->map('GET', '/', 'home', 'Home');
$router->map('GET', '/contact', 'contact', 'Contact');
$router->map('GET', '/about', 'about', 'About');
$router->map('GET', '/[*:slug]/[i:id]', 'article', 'Article');

$router->map('GET', '/login', function() {
    require './controller/users/login.php';
}, 'Login');
$router->map( 'POST', '/login', function() {
    require './controller/users/login.php';
});

$router->map('GET', '/forgot-password', function() {
    require './controller/users/forgotPassword.php';
}, 'Forgot');

$router->map( 'POST', '/forgot-password', function() {
    require './controller/users/forgotPassword.php';
});

$router->map('GET', '/logout', 'logout', 'Logout');

$router->map('GET', '/register', function() {
    require './controller/users/register.php';
}, 'Register');
$router->map( 'POST', '/register', function() {
    require './controller/users/register.php';
});

$router->map('GET', '/account', function() {
    require './controller/users/account.php';
}, 'Account');

$match = $router->match();

// call closure or throw 404 status
if(is_array($match)) {
    require './views/inc/header.phtml';

    if (is_callable( $match['target'])) {
        call_user_func_array( $match['target'], $match['params'] );
    } else {
        $params = $match['params'];
        require "./controller/{$match['target']}.php";
    }

    require './views/inc/footer.phtml';
} else {
    // no route was matched
    header( $_SERVER["SERVER_PROTOCOL"] . ' 404 Not Found');
    require './views/404.php';
}

I looked around on stack overflow but every post relates to a database object.

I do not understand what can affect the value of $pdo after implementing a router. Do i need to create a class to solve this problem?

Any help is appreciated.

tereško
  • 58,060
  • 25
  • 98
  • 150
Biscuite
  • 85
  • 9
  • 2
    Where you load the controller is the problem.. It is no doubt in a function or method and so `$pdo` is not global. Don't use globals, pass in `$pdo` or something. – AbraCadaver Mar 18 '20 at 18:02
  • @AbraCadaver What do you mean by "pass in $pdo" ? – Biscuite Mar 18 '20 at 19:04
  • Not the best way but probably the quickest fix would be `$GLOBALS['pdo'] = $pdo;` at the end of `connect.php`. – AbraCadaver Mar 18 '20 at 19:19
  • You would need to show router code and how it loads the controller for a better solution. – AbraCadaver Mar 18 '20 at 19:25
  • @AbraCadaver I've added the router code so you can see how every controller is loaded. Your quick fix is working, but if you tell me their is a better solution i'll be glad to ear it. – Biscuite Mar 18 '20 at 19:49

1 Answers1

1

@AbraCadaver already explained you the reason, why you encountered the specified prolem. So, in this regard, all credits go to him.

Note that, in a correct implementation of an MVC application, a database connection is used only by the components of the model layer.

So, having your current design in mind, the correct solution is to pass the $pdo instance to each model object created in the scope of the matched route's target.

So, UsersModel should receive $pdo as dependency:

class UsersModel {

    /**
     * Database connection.
     * 
     * @var \PDO
     */
    private $connection;

    /**
     * @param PDO $connection Database connection.
     */
    public function __construct(\PDO $connection) {
        $this->connection = $connection;
    }

    public function confirmedUser($username) {
        // ...Please, no global variables anymore!...

        $sql = 'SELECT * 
                FROM users 
                WHERE 
                    username = :username AND 
                    confirmed_at IS NOT NULL';

        $statement = $this->connection->prepare($sql);

        $statement->execute([
            ':username' => $username,
        ]);

        $data = $statement->fetch(PDO::FETCH_ASSOC);

        /*
         * As I recall, PDOStatement::fetch returns FALSE when no records are found - instead of 
         * an empty array, like PDOStatement::fetch returns. But, in terms of PDO, a returned 
         * FALSE means "failure" and triggers an exception.
         * So I handle this "special" situation here.
         */
        return ($data === false) ? [] : $data;
    }
}

The controller (like path-to/controller/users/register.php) should then have to pass the $pdo object to the newly created UsersModel instance (see Dependency injection):

<?php
require_once '../php/connect.php';
require_once '../models/UsersModel.class.php';

$usersModel = new UsersModel($pdo);

if(/*...*/) {
    //...

    $usersModel->confirmedUser($username);
}

Suggestion: Always use require instead of require_once when including a file in which the database connection is created:

require "../php/connect.php";
PajuranCodes
  • 303
  • 3
  • 12
  • 43
  • 1
    Suggestion: Always use *autoloading* and forget about including any file manually – Your Common Sense Mar 19 '20 at 08:21
  • Thanks to your help it's working like it did before i implement that router. I know understand that i should not declare $pdo as a global variable. I will read your post about the Model layer and the wiki that talk about dependency injection – Biscuite Mar 19 '20 at 17:19
  • @YourCommonSense Actually, I avoided on purpose to give further suggestions, such as yours. But it's definitely welcome. – PajuranCodes Mar 19 '20 at 17:53
  • @Biscuite You are welcome. Maybe you'll find [this](https://symfony.com/doc/current/create_framework/index.html), [this](https://www.youtube.com/watch?v=Ojsn11XY0X8), [this](https://vimeo.com/107963074) and [this](https://www.youtube.com/watch?v=RlfLCWKxHJ0) helpful. – PajuranCodes Mar 19 '20 at 18:02