Proper usage of OOP PHP

Question

I am new to the OOP world I have been reading as much as I can about it and have never been more confused. I understand that its great for organizing code and making it more maintainable, etc. I have written some OOP code but I am unsure if its proper, it works fine though.

I am confused about public private functions along with extends and constructors. I haven't used them yet in my code and do not see where I would use them in a real life example. The more I read and try to understand it the more confused I get. I have included my code and was wondering if someone could point out mistakes, improvements, proper usage, organization. I feel it would help me better understand if someone with a lot of experience was to look at my code and give me pointers.

class userFunctions {

  const SALT_LENGTH = 9;

  //Retrieves encrypted password from database returns in variable $salt
  public function retrievePassword($conn,$username) {
    try{
      $stmt = $conn->prepare('SELECT `password` FROM `users` WHERE `userName`= :userName');
      $stmt->bindValue(':userName', $username);
      $stmt->execute();
      $salt = $stmt->fetchColumn();

        } catch (PDOException $e){
            echo 'Connection failed: ' . $e->getMessage();
        }
            return $salt;
    }

  //Generate an encrypted password method
  public function generateHash($password, $salt = null) {
    if ($salt === null) {
      $salt = substr(md5(uniqid(rand(), true)), 0, self::SALT_LENGTH);
    } else {
      $salt = substr($salt, 0, SALT_LENGTH);
    }
    return $salt . sha1($salt . $password);
 }

//Check database for duplicate username
  public function userCheck($conn,$userName) {
    try{
      $stmt = $conn->prepare('SELECT COUNT(*) FROM `users` WHERE `userName` LIKE CONCAT("%",:userName)');
      $stmt->bindValue(':userName', $userName);
      $stmt->execute();
      $count = $stmt->fetchColumn();
      return $count;
    } catch (PDOException $e){
      echo 'Connection failed: ' . $e->getMessage();  
    }
  }

  //Add user to database
  public function Register($conn,$userName,$encryptedPass) {
    try{
      $stmt = $conn->prepare('INSERT INTO users (userName, password) VALUES (:userName, :password)');
      $stmt->execute(array(':userName' => strip_tags($userName), ':password' => $encryptedPass));
    }  catch (PDOException $e){
      echo 'Connection failed: ' . $e->getMessage();
    }    
  }
}

Answer 1

Right, your object is, like Rene said, just a container of functions. Though, in theory, that's what objects are for (group functionality together), the way you went about this, you're actually using classes as modules. It's not what you call SOLID code.

Edit:
Perhaps you might want to scroll down first, because throughout the answer I've used rather messy examples to explain things, but at the bottom I've added a really simple example that might help you to understand the access modifiers and inheritance better.

Anyhow, I'm not going into details on how you should refactor this to be SOLID compliant, that's part of the learning curve. The simple fact of the matter is that, yes OOP is confusing at first, and at first you might not even need constructors, access modifiers (public, protected or private), let alone destructors. Especially if you haven't come to terms with inheritance. But let me explain a few things that, hopefully, will clarify one or two things.

Class methods share state:
That's just a fancy way of saying that class methods have access to variables, without having them passed as arguments. Of course, not just any variable will do: it's the properties of the class that are accessible. In your snippet, you're passing $conn to various methods, like you would if they were functions. The difference is, that classes can be assigned properties, too.

class RefactoredFunctions
{
    private $conn = null;
    public function setConn($conn)
    {
        $this->conn = $conn;
    }
    public function retrievePassword($userNAme)
    {
        //your code here, but replace $conn with $this->conn
    }
}

Now you can call the retrievePassword method, without having to pass the $conn param over and over again. Though, this code would fail, if $conn wasn't set prior to calling retrievePassword. How can we fix that?
Enter constructors. A constructor is a method, like any other, except for the fact that it can't be called manually. It's called automatically, when an instance of the class is created. You could define a constructor that expects an argument, and pass the connection to it, ensuring the $conn property will be set:

public function __constructor($conn)
{
    $this->conn = $conn;
}
//using this class:
$instance = new RefactoredFunctions($conn);
$instance->retrievePassword('userFoo');//conn is set through constructor!

That's all a constructor does (9/10 times): setting properties your class requires to work properly. These properties are sometimes called the dependencies of that class. The class depends on them.
Now, an argument that is called $conn is clearly supposed to be a DB connection in your case, but how do we make sure nobody passes a string? Simple: we use type-hinting, another benefit of OOP:

public function __construct(PDO $conn)
{
    $this->conn = $conn;
}
//usage
$instance= new RefactoredFunctions(new PDO());//works fine
$error = new RefactoredFunctions('foobar');//fatal error string is not PDO instance

This makes your code more failsafe. Use type-hints to avoid bugs. That's an order!
Now, all this time, I've been using $this->conn which is a property declared as private. Why? Simple. I've used type-hints to ensure the value of $this->conn is guaranteed to be an instance of PDO, but if I would have declared public $conn, I'm able to access the property (and thus assign it a new value) simply bypassing the setter/constructor:

//assume public $conn
$instance= new RefactoredFunctions(new PDO());//works fine
$instance->retrievePassword('userFoo');//still OK
$instance->conn = 'reassign conn';
$instance->retrievePassword('userFoo');//ERROR!

Now, with private properties:

$instance= new RefactoredFunctions(new PDO());//works fine
$instance->conn = 'reassign conn';//ERROR!!
$instance->retrievePassword('userFoo');//will never get this far

What's the benefit of this? Simple: the bug is easy to find. Imagine if I would've reassinged the conn property by accident, and then call a method 50 lines further down the script, or pass an instance to another class (defined in another file), and call the method there. I'd get an error, with a file & line number that contains the call, not the cause (reassignment statement).

OK, you got this far, and this is still making sense? On to protected... Where does that tie in?
Well, the more objects you have, chances are some of them share functionality. Rather than writing the same methods several times, we extend classes from each other. So far, we have:

class RefactoredFunctions
{
    private $conn = null;
    public function __construct(PDO $conn)
    {
        $this->conn = $conn;
    }
    public function setConn(PDO $conn)
    {
        $this->conn = $conn;
        return $this;
    }
    public function retrievePassword($userNAme)
    {
        //your code here, but replace $conn with $this->conn
    }
}

Now, suppose I wanted a DB class per table (quite common). I have this base class, that holds a connection instance, and some methods to query a DB. What if I moved the constructor, $conn property and setConnection method to a generic class, and create child classes per table? that would save me 2 methods/class to write. If I have 10 tables, that's 20 methods. Of course I'll do that:

class BaseTable
{
    private $conn = null;
    private $tblName = null;//to hold the table their linked to

    public function __construct(PDO $conn)
    {
        $this->conn = $conn;
    }

    public function setConn(PDO $conn)
    {
        $this->conn = $conn;
        return $this;
    }

    public function setTable($tblName)
    {//set table name for each instance
        $this->tblName = $tblName;
        return $this;
    }
}
//a child
class UserTbl extends BaseTable
{
    public function getUsers()
    {
        $this->conn->prepare('SELECT * FROM '.$this->table);
        //etc...
    }
}
//usage:
$users = new UserTbl(new PDO());
$users->getUsers();

Ok, so I didn't have to write those properties and methods, but the code above won't work! Why? because private means the properties/methods are only visible to the class in which they were defined, in this case BaseTable, not the child class. protected means that properties/methods can be accessed in both the class that defined them, and its children. So, would you make the $conn property protected? Of course NOT.
There's a number of reasons why you wouldn't do this, but I might go into those in a future edit. ATM, we have some more important things to focus on.

Have you noticed how the connection is guaranteed to be available thanks to the constructor, but the tablename isn't? yet, we're using the table name in our queries. How do we fix that? You have 3 options:

1. Make $tblName protected and assign it a value in the child's definition, and remove the setTable method
2. override the parent constructor (less secure)
3. A combination of the first two

So, if in the base class, we now have protected $tblName = null; you can write this, in the child:

protected $tblName = 'users';

Job done. The second approach would look like this:

//in child
public function __construct(PDO $connection, $tblName)
{
    $this->tblName = $tblName;
    parent::__construct($connection);//explicit call required
}

To combine the two would require using abstract, final and all that, here's an example of this, but I'm not going into all that now.

All this time, we still have to deal with the private $conn being invisible to the child classes. How would you deal with that? The answer to that question is surprizingly simple:

//in BaseTable
protected function getConnection()
{
    return $this->conn;
}

This protected method is visible to the children, and returns the connection, so all we have to do now is write:

$this->getConnection()->prepare('SELECT ...');

instead of $this->conn. There we are, problem solved, you should be able to get these classes working now.
Note that there's a lot of things left to be done. For example the getUsers methods is nothing but a select * from tbl query. We could just as well have written that in our BaseTable class, but using another name and access modifier:

protected function getAll()
{//protected: it's for internal use only
    $res = $this->conn('SELECT * FROM '.$this->tblName);
    return $res->fetchAll();
}

Then, in each child class:

//UsersTable:
public function getUsers()
{
    return $this->getAll();
}
//LogsTable
public function getLogs()
{
    return $this->getAll();
}

You could even add arguments in the child methods, and process the return value of the getAll method accordingly:

protected function getAll()
{
    return $this->conn->query('SELECT * FROM '.$this->table);
}
//UsersTable:
public function getUsers($asArray = false)
{
    $asArray = $asArray ? PDO::FETCH_ASSOC : PDO::FETCH_OBJ;
    return $this->getAll()->fetchAll($asArray);
}

One last thought on inheritance: Assume the classes BasteTable and UserTable extends BaseTable. When using type-hinting, this is good to know:

$base = new BaseTable();
$user = new UserTable();
if ($base instanceof BaseTable) echo 'Yes';//will echo, of course
if ($user instanceof UserTable) echo 'Duh';//of course, this is true

That's as you'd expect it, but:

if ($base instanceof UserTable) echo 'WTF';//You won't see this
if ($user instanceof BaseTable) echo 'Yes';//will echo!!

So all instances of UserTable are instances of BaseTable, too, but not all instances of BaseTable are instances of UserTable. Now this might seem odd, at first but think of it like this:

class Room
{
    public $door;//can be opened from outside
    protected $window;//can only be opened from within ANY room, all rooms have to access this
}
class Bathroom extends Room
{
    private $mirror;//this room has a mirror, can only be accessed in a bathroom
}
class Bedroom extends Room
{
    private $bed;//this one has a bed, can only be used when inside a bedroom
}

Now you see: both a bedroom and a bathroom are rooms, but a bedroom is no bathroom, hence:

$bath = new Bathroom;//instanceof Room, not instanceof Bedroom
$bed = new Bedroom;//instanceof Room, not instanceof Bathroom
$genericRoom = new Room;//instanceof Room
//not specified which, so no mirror or bed, so not instance of any child

I hope this clears one or two things up for you. If you have a follow up question, don't hesitate to ask. I'm not going to add any more info, as this is quite a lot to deal with already

Answer 2

What you're really doing here, is using a class as a container for bunch of functions. It's a perfectly fine thing to do, but it's not really object-oriented programming. Even the name of your class userFunctions confirms it.

An object should be something that binds together some state and a group of methods which operate on this state. In your case most of the methods need a database connection object and a username to do their work, so this should be your shared state:

class User {
    private $db;
    private $username;

    function __construct($db_connection, $username) {
        $this->db = $db_connection;
        $this->username = $username;
    }

    public function exists() {
        ...
    }

    public function register($pass) {
        ...
    }
}

So here we have a User class. We instantiate it with a database connection and a username, and then we have an object that represents one particular user, and we can call the various methods without having to pass in the connection and username to each one of them.

We also can then work at a higher level, having a concept of a User, we can tell it to do various tasks:

$user = new User($db_connection, $_POST["user"]);
if (!$user->exists()) {
    $user->register($_POST["pass"]);
}

Additional tips:

• Indent your source code properly.
• Follow PHP naming convention of starting class names with uppercase letter and method names with downcase letter.
• Drop the ///////////// separators - whitespace is enough.