PHP5 OOP Class Structure

Question

I know there are loads of questions on this, I have done quite a bit of reading. I'd like to ask this in context of my project to see what suggestions you may have.

I have quite a large web application with many classes, e.g. users and articles (which i consider to be the main classes) and smaller classes such as images and comments. Now on a page, lets say for example an article, it could contain many instances of images and comments. Makes sense right? Now on say an articles page I call a static method which returns an array of article objects.

That's the background, so here are the questions.

Since building a large amount of the app I came to realise it would be very useful to have a core system class containing settings and shared functions. There for I extended all of my classes with a new core class. Seemed relatively simple and quick to implement. I know CodeIgniter does something similar. I feel now though my app is becoming a bit messy.

Question Is this a good idea? Creating an instance of core is exactly what I want when calling an instance of an article, but what about when i'm creating multiple instances using the static method, or calling multiple images or comments on a page. I'm calling the core class unnecessarily right? Really it only needs to be called once per page (for example the constructor defines various settings from the database, I don't want to this every time, only once per page obviously), but all instances of all classes should have access to that core class. Sounds exactly like I want the singleton approach, but I know that's a waste of time in PHP.

Here's an idea of what my code looks like at this point. I've tried to keep it as simple as I can.

class core {

    public function __construct(){
        ...define some settings which are retrieve from the database
    }

    public function usefulFunction(){
    }
}


class user extends core {

    public function __construct(){
        parent::__construct();
    }

    public function getUser($user_id){
        $db = new database();
        $user = /* Get user in assoc array from db */

        $this->__setAll($user);
    }

    public static function getUsers(){
        $db = new database();
        $users = /* Get users from database in assoc array from db */

        foreach($users as $user) {
             $arrUsers[] = new self();
             $arrUsers[]->__setAll($user);
        }

        return $arrUsers;
    }

    private function __setAll($attributes) {
        foreach($attributes as $key => $value)
        {
            $this->__set($key, $value);
        }   
    }

    public function __set($key, $value) {
         $this->$key = $value;
    }

}

The other issue I'm having is efficiently using/sharing a database connection. Currently each method in a class requiring a database connection creates a new instance of the database, so on a page I might be doing this 5 or 10 times. Something like the dependency injection principle sounds much better.

Question Now if i'm passing the instance of the DB into the new user class, i know I need something like this...

class user{
    protected $db;

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

    ... etc
}

$db = new database();
$user = new user($db);

... but when I want to run the static function users::getUsers() what is the best way to gain access to the database instance? Do i need to pass it as a variable in each static method? (there are many static methods in many classes). It doesn't seem like the best way of doing it but maybe there isn't another way.

Question If extending all of my classes off the core class as suggested in part 1, can I create an instance of the DB there and access that some how?

Question I also have various files containing functions (not oop) which are like helper files. What's the best way for these to access the database? Again i've been creating a new instance in each function. I don't really want to pass the db as a parameter to each one. Should I use globals, turn these helper files into classes and use dependency injection or something different all together?

I know there is lots of advice out there, but most info and tutorials on PHP are out of date and don't ever seem to cover something this complex...if you can call it complex?

Any suggestions on how to best layout my class structure. I know this seems like a lot, but surely this is something most developers face everyday. If you need any more info just let me know and thanks for reading!

Answer 1

You can start with an abstract class that handles all of your Database queries, and then constructs them into objects. It'll be easy to set yourself up with parameterized queries this way, and it will standardize how you interact with your database. It'll also make adding new object models a piece of cake.

http://php.net/manual/en/language.oop5.abstract.php

abstract class DB
{
  abstract protected function table();
  abstract protected function fields();
  abstract protected function keys();

  public function find()
  {
    //maybe write yourself a parameterized method that all objects will use...
    global $db; //this would be the database connection that you set up elsewhere.
    //query, and then pack up as an object
  }

  public function save()
  {
  }

  public function destroy()
  {
  }
}

class User extends DB
{
  protected function table()
  {
    //table name
  }  

  protected function fields()
  {
    //table fields here
  }

  protected function keys()
  {
    //table key(s) here
  }

  //reusable pattern for parameterized queries
  public static function get_user( $id )
  {
    $factory = new User;
    $params = array( '=' => array( 'id' => $id ) );
    $query = $factory->find( $params );
    //return the object
  }
}

You'll want to do your database connection from a common configuration file, and just leave it as a global variable for this pattern.

Obviously this is just scratching the surface, but hopefully it gives you some ideas.

Answer 2

You asked in a comment that I should elaborate why it is a bad idea. I'd like to highlight the following to answer that:

Ask yourself if you really need it.

Do design decisions for a need, not just because you can do it. In your case ask yourself if you need a core class. As you already have been asked this in comments you wrote that you actually do not really need it so the answer is clear: It is bad to do so because it is not needed and for not needing something it introduces a lot of side-effects.

Because of these side-effects you don't want to do that. So from zero to hero, let's do the following evolution:

You have two parts of code / functionality. The one part that does change, and the other part that is some basic functionality (framework, library) that does not change. You now need to bring them both together. Let's simplify this even and reduce the frame to a single function:

function usefulFunction($with, $four, $useful, $parameters)
{
    ...
}

And let's reduce the second part of your application - the part that changes - to the single User class:

class User extends DatabaseObject
{
    ...
}

I already introduced one small but important change here: The User class does not extend from Core any longer but from DatabaseObject because if I read your code right it's functionality is to represents a row from a database table, probably namely the user table.

I made this change already because there is a very important rule. Whenver you name something in your code, for example a class, use a speaking, a good name. A name is to name something. The name Core says absolutely nothing other that you think it's important or general or basic or deep-inside, or that it's molten iron. No clue. So even if you are naming for design, choose a good name. I thought, DatabaseObject and that was only a very quick decision not knowing your code even, so I'm pretty sure you know the real name of that class and it's also your duty do give it the real name. It deserves one, be generous.

But let's leave this detail aside, as it's only a detail and not that much connected to your general problem you'd like to solve. Let's say the bad name is a symptom and not the cause. We play Dr. House now and catalog the symptoms but just to find the cause.

Symptoms found so far:

• Superfluous code (writing a class even it's not needed)
• Bad naming

May we diagnose: Disorientation? :)

So to escape from that, always do what is needed and choose simple tools to write your code. For example, the easiest way to provide the common functions (your framework) is as easy as making use of the include command:

 include 'my-framework.php';    
 usefuleFunction('this', 'time', 'really', 'useful');

This very simple tow-line script demonstrates: One part in your application takes care of providing needed functions (also called loading), and the other part(s) are using those (that is just program code as we know it from day one, right?).

How does this map/scale to some more object oriented example where maybe the User object extends? Exactly the same:

include 'my-framework.php';
$user = $services->store->findUserByID($_GET['id']);

The difference here is just that inside my-framework.php more is loaded, so that the commonly changing parts can make use of the things that don't change. Which could be for example providing a global variable that represents a Service Locator (here $services) or providing auto-loading.

The more simple you will keep this, the better you will progress and then finally you will be faced with real decisions to be made. And with those decisions you will more directly see what makes a difference.

If you want some more discussion / guidance for the "database class" please consider to take a read of the very good chapter about the different ways how to handle these in the book Patterns of Enterprise Application Architecture which somewhat is a long title, but it has a chapter that very good discusses the topic and allows you to choose a fitting pattern on how to access your database quite easily. If you keep things easy from the beginning, you not only progress faster but you are also much easier able to change them later.

However if you start with some complex system with extending from base-classes (that might even do multiple things at once), things are not that easily change-able from the beginning which will make you stick to such a decision much longer as you want to then.

Answer 3

Summarize all answers:

1. Do not use single "God" class for core.
2. It's better to use list of classes that make their jobs. Create as many class as you need. Each class should be responsible for single job.
3. Do not use singletones, it's old technique, that is not flexible, use dependecy injection container (DIC) instead.

Answer 4

First, the the best thing to do would be to use Singleton Pattern to get database instance.

class Db{
protected $_db;

private function __construct() {
       $this->_db = new Database();
}

public static function getInstance() {
if (!isset(self::$_db)) {
    self::$_db = new self();
}
return self::$_db;
}
}

Now you can use it like db::getInstance(); anywhere.

Secondly, you are trying to invent bicycle called Active Record pattern, in function __setAll($attributes).

In third, why do you wrote this thing in class that extends Core?

public function __construct(){
    parent::__construct();
}

Finally, class names should be capitalized.