6
\$\begingroup\$

I was hoping if anyone could tell me if I'm doing this correctly, I know there are other questions like this but everyone codes different and I just feel that I'm not doing this right. I'm trying to make a modular OOP site so all the files will be called through the one index file which will then route the user to the correct page. All the code below works correctly (I have tested it) but I was curious about a few things.

  • Is this the correct way of using OOP and classes (mainly the DB connection)?
  • Why (if it is correct) when I use Netbeans PHP that it doesn't pick up the functions? See images below.

Netbeans showing examples

Netbeans not showing examples from user class

db.php (DB connection file)

$host = "localhost";
$username = "root";
$password = "";
$database = "als";

$errors = new errors();

try {
    $db = new PDO("mysql:host=$host;dbname=$database", $username, $password);
} catch(PDOException $ex) {
    trigger_error($ex->getMessage());
    $errors->addLog($ex->getPrevious(), true);
    exit;
}

User.php (class)

class user {
    private $db = null;

    public function __construct() {
        global $db;
        $this->db = $db;
    }

    public function deleteUser($ID) {      
        if(isset($ID) && !empty($ID) && is_int($ID) == TRUE) {
            $deleteUser = $this->db->prepare("DELETE FROM `users` WHERE userid=?");
            $deleteUser->bindParam(1, $ID, PDO::PARAM_INT);
            $deleteUser->execute();
        } else {
            return FALSE;
        }     
    }
}

index.php (main file)

include 'config/autoloader.php';
include 'config/db.php';

$user = new user();

$user->deleteUser(1);
\$\endgroup\$
  • 1
    \$\begingroup\$ May I suggest using PHPStorm rather than NetBeans - would make your life much easier as PHPStorm has better code analysis and autocomplete features. \$\endgroup\$ – jsanc623 Sep 26 '14 at 21:38
  • \$\begingroup\$ @jsanc623 believe it or not, I just removed PHPStorm half an hour before I posted this question because of the same reason. I thought the IDE was messed up. Now Netbeans is doing it too so I guess something else is wrong. Any ideas? \$\endgroup\$ – Script47 Sep 26 '14 at 21:47
1
\$\begingroup\$

Why (if it is correct) when I use Netbeans PHP that it doesn't pick up the functions? See images below.

As you don't declare types in PHP, Netbeans has no way of knowing that $db is a PDO object. But you can tell Netbeans this:

/* @var $db PDO  */
private $db = null;

For further ways to Type-hint for PHP in Netbeans check out this post I wrote a while ago.

Is this the correct way of using OOP and classes (mainly the DB connection)?

I would pass the PDO object to the user class in the constructor instead of using global.

It looks cleaner, and it would make writing unit tests a lot easier as well (you could pass a connection to a test database without changing the code or mock the PDO class). It also has the neat side-effect that you are able to define the type and thus enable auto complete.

Misc

  • classes should start with an upper-case letter (User).
  • is_int($ID) == TRUE can be written as is_int($ID).
  • I find it a bit odd that you return false for wrong parameters, but not for an unsuccessful delete in deleteUser.
  • checking empty has the effect of not deleting the user with id 0. Is this on purpose? If yes, I would make this explicit ($id != 0), if no, just remove it, it doesn't do anything else that is_int wouldn't cover.

So I would probably rewrite it like this:

class User { // upper-case U
    private $db = null;

    public function __construct(PDO $db) { // defining a type allows for auto-complete and makes testing easier
        $this->db = $db;
    }

    public function deleteUser($ID) {      
        if(!isset($ID) || !is_int($ID)) { // no need to check empty. if needed, add $ID != 0
            return FALSE; // wrong parameters, return early
        }
        $deleteUser = $this->db->prepare("DELETE FROM `users` WHERE userid=?");
        $deleteUser->bindParam(1, $ID, PDO::PARAM_INT);
        return $deleteUser->execute(); // returns true on success, otherwise false

    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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