0

So I've learnt a lot about PHP, and I started with the basics even if some of it is deprecated. Now I've got the knowledge down, I'm re-writing everything I've done to use PDO and class'. Now I've never used either but I'm slowly (slowly, slowly) understanding and getting my head around it. What I'd like to ask, is if I'm doing this right, if not where am I going wrong?

I basically have this function:

function user_exists($username) {
    $username = sanitize($username);
    $query = mysql_query("SELECT COUNT(`user_id`) FROM `users` WHERE `username` = '$username'");
    return (mysql_result($query, 0) == 1) ? true : false;
}

And I want to turn it into a class:

class user {
    function exists($username) {
        $username = $username;
        $query = $db->query("SELECT COUNT(`user_id`) FROM `users` WHERE `username` = '$username'");
        $results = $query->fetchAll(PDO::FETCH_ASSOC);
    }
}

Am I on the right track, should I be making the variables in that function (method) private to the "exists" method?

I plan to make a "users" class, with methods such as exists, banned, admin, ect. I think this is what I should be doing?

Thanks if anyone answers this.

  • `$username = $username;` is _particularly_ wrong. First it has no effect, second it's bad naming. Maybe you meant `$this->name = $name;`, where `name` is a class property. – Sergiu Paraschiv Feb 05 '14 at 15:37
  • Correct me if I'm wrong but a 'property' in a class just means variable right? Like 'method' means function? What I had in my functions, is when the user logs in it checks if that username exists, so it parses it into the function to be used in the query. So is what you're saying the right thing to do? Also, if you can answer; are pdo querys safe? Only inserts, ect are prone to sql injection and should be prepared right? – user3245541 Feb 05 '14 at 15:41

2 Answers2

1
class User {
    public $name;

    private $db;

    function __construct($name) {
        $this->name = $name;

        $this->db = get_me_a_db_connection();
    }

    public function exists() {
        $query = $this->db->query('SELECT COUNT(user_id) FROM users WHERE username = "' . $this->name . '"');
        $results = $query->fetchAll(PDO::FETCH_ASSOC);

        return some_code_that_extracts_the_count_from_$results;
    }
}

But: 1) $db should not be a class property. It should at least be in a factory. So you extract it in a factory:

class Factory {
    public static function db() {
        return get_me_a_db_connection();
    }
}

public function exists() {
    $query = Factory::db()->query(...);
    ...
}

This way you don't pollute your domain models with infrastructure code that much.

2) By that much above I mean you can achieve total decoupling between infrastructure code (database connection, querying) and your domain models. There are tons of libraries out there doing just that. I'd start with Laravel's Eloquent.

3) PDO queries are safe if you use prepared statements. All the libraries like the ones I linked to above use prepared statements. As a noob I would recommend you start learning by studying one of them.

Community
  • 1
  • 1
Sergiu Paraschiv
  • 9,929
  • 5
  • 36
  • 47
  • Thanks this is really helpful, thank you for explaining. I'll take a look at using that library; I've never used one before but it look's like it takes away a lot of the bulk, say a query and puts it into models? I think I need to read more.. but I'm getting it slowly. Thanks again. – user3245541 Feb 05 '14 at 16:07
0

Maybe you're not too much out of the track, and that's a good start. But you need to learn how about to enter in what is Objected-oriented Programming is. I would start with a simple class such as User with the information of a real user, whatever the information this user needs for all the methods you're trying to create.

The private variables you will need always are the informations you will need for various methods. Like the username, and any other information you need to find out.

Maybe $results could be a variable with the information of the last return, or just return it like your first example.

You need to first think in what you really need, before writing down your class.

Hotwer
  • 150
  • 9
  • So you're saying to make a class, with everything statically set? So my class would have like $this->username = $username, $this->first_name = $first_name and all parsed into the method user($username, $first_name)? I've read a lot into OOP, it's just taking my brain a good deal of thinking to get my head around it as it's a pretty big change from older styles. I learn best by trial and error, if I come across of trying to run before I can walk. – user3245541 Feb 05 '14 at 15:54
  • Maybe it's just my way-to-go in programming. I usually do everything, not in a try and error way. Sometimes it's more satisfying and always, and that's the most important part, is that you know how the WHOLE thing works, and it really helps solving problems and making further changes in your program. – Hotwer Feb 05 '14 at 16:53
  • This is not OOP. OOP does not require classes. Writing code in classes does not guarantee you are doing OOP. Start by making this distinction. – Sergiu Paraschiv Feb 07 '14 at 08:35