0

I have setup a pdo connection and pass that as a variable into a function. This all works fine and the function returns correctly. If I run the function in a conditional statement with the PDO variable and a name it runs correctly - if name is in database it echos correctly if not then it also echos correctly. What I want to do is to pass the value of a form post to the function so that it checks to see if it exists in the database. Here is my code:

The function checks to see if the column count is one.

function user_exists($pdo, $username) {
$stmt = $pdo->prepare('SELECT COUNT(uid) FROM users WHERE username =    :username');
$stmt->execute(['username' => $username]);
$result = $stmt->fetchColumn();
return ($result == 1);
}

If the admin user exists in the database echo 'exists' - Just for testing.

if(user_exists($pdo,'admin') == true) {
echo "exists";
} else {
echo "doesnt exist";
}

Checks to see of both fields have been entered then I want it to check if the username entered is in the database, but I am doing something wrong.

if(!empty($_POST) === true) {
$username = $_POST['username'];
$pwood = $_POST['password'];
if(empty($username) === true || empty($pwood) === true) {
echo "You need to enter a username and password";
} else if (user_exists($pdo,$_POST['username']) == false){
echo 'We can\'t find that username. Please try again or register';
}
}
tinamou_ed
  • 33
  • 6
  • 1
    this line `if(!empty($_POST) === true) {` – Rotimi Oct 17 '17 at 20:29
  • Perfect thanks very much. Im not sure why though. The value of post is not empty.... – tinamou_ed Oct 17 '17 at 20:35
  • 1
    @GrumpyCrouton, using `global` variables is *not* recommended. See https://stackoverflow.com/questions/5166087/php-global-in-functions or https://stackoverflow.com/questions/1557787/are-global-variables-in-php-considered-bad-practice-if-so-why – Bill Karwin Oct 17 '17 at 20:42
  • note on @GrumpyCrouton `global` is be easy for beginners, but becomes very "evil" in bigger projects when using with classes. – clemens321 Oct 17 '17 at 20:42
  • @BillKarwin Thanks, I wasn't aware of that. Is there a better way to do it where you don't have to pass it is a parameter every time you call the function? – GrumpyCrouton Oct 17 '17 at 20:43
  • @GrumpyCrouton, use OO programming. Create a class for the methods that use your pdo object, and set the pdo resource as a member of that class. – Bill Karwin Oct 17 '17 at 20:44
  • @BillKarwin Yeah I guess this could pretty easily be turned into a user class or something. That makes sense. – GrumpyCrouton Oct 17 '17 at 20:44
  • Good stuff. I'm a beginner and its all really helpful to know. – tinamou_ed Oct 17 '17 at 21:24

1 Answers1

2

Better don't compare with bool values, just use

//...see below
require_once('UserRepository.php');
$ur = new UserRepostory($pdo);

if(!empty($_POST)) {
    if (empty($username) || empty($pwood)) {
        // something
    } else if (!$ur->exists($_POST['username'])) { // your user_exists($pdo, $username) works fine, too
        // something else
    }
}

Especially the initial if(!empty($_POST) === true) { is hard to read and lead to errors 'cause of misunderstood operator priority.

Update based on the comments above, here is an example with a class:

// UserRepository.php
class UserRepository {
    private $pdo;

    // '\PDO' instead of 'PDO', see PHP Namespacing
    public function __construct (\PDO $pdo)
    {
        $this->pdo = $pdo;
    }

    public function exists($username)
    {
        $sql = 'SELECT COUNT(uid) FROM users WHERE username = :username');
        $stmt = $this->pdo->prepare($sql);
        $stmt->execute(['username' => $username]);
        $result = $stmt->fetchColumn();

        return (bool) $result;
    }
}
clemens321
  • 1,953
  • 9
  • 18
  • I can see how that makes sense. OO is a bit beyond my skill level at the moment. I'm sure Ill come back to this answer though. – tinamou_ed Oct 17 '17 at 21:27
  • As mentioned in the code comment, the function without class works fine and is good for a small project and beginners, too. There is nothing bad in starting small, it's just to show a "right" way, and not to use a global variable. – clemens321 Oct 17 '17 at 21:36
  • Really great edit, I wish I could upvote a second time. – GrumpyCrouton Oct 18 '17 at 00:09